<br><br><div class="gmail_quote">On Tue, Feb 15, 2011 at 1:10 PM, Nicolas Cellier <span dir="ltr">&lt;<a href="mailto:nicolas.cellier.aka.nice@gmail.com">nicolas.cellier.aka.nice@gmail.com</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
2011/2/15 Eliot Miranda &lt;<a href="mailto:eliot.miranda@gmail.com">eliot.miranda@gmail.com</a>&gt;:<br>
<div><div></div><div class="h5">&gt;<br>
&gt;<br>
&gt; On Tue, Feb 15, 2011 at 12:39 PM, Stéphane Ducasse<br>
&gt; &lt;<a href="mailto:stephane.ducasse@inria.fr">stephane.ducasse@inria.fr</a>&gt; wrote:<br>
&gt;&gt;<br>
&gt;&gt; Eliot<br>
&gt;&gt;<br>
&gt;&gt; I can understand that well. Now we could just let this code in VMMaker and<br>
&gt;&gt; not inlining.<br>
&gt;&gt; We fix 8 users and we are done. I have concerned that we have a complex<br>
&gt;&gt; solution because it is complex<br>
&gt;&gt; for a couple of use case. Of course we can do nothing (and we will<br>
&gt;&gt; probably not do it now) but in that case we can<br>
&gt;&gt; also stop pharo right now because a large part of the system could follow<br>
&gt;&gt; the exact same principle.<br>
&gt;<br>
&gt; That&#39;s a false distinction.  There&#39;s nothing wrong with caseOf: and not<br>
&gt; addressing it will not leave Pharo any the worse.  Case statements are not<br>
&gt; in themselves evil in the right context.  The main problems with the<br>
&gt; index/dictionary approach are that<br>
&gt; a) the solution is no longer contained in a single method<br>
&gt; b) the dictionary is metadata that needs to be updated in an initialize<br>
&gt; method; forget to initialize the dictionary when an index changes and your<br>
&gt; program is broken.<br>
&gt; As my first wife said &quot;don&#39;t sweat the petty stuff; pet the sweaty stuff&quot;.<br>
&gt;<br>
&gt; BTW, there&#39;s a false assumption that has been stated in this thread that the<br>
&gt; caseOf: statement is only used with explicit integers.  It can be used with<br>
&gt; anything.  Expressions, class constants etc.<br>
&gt;&gt;<br>
&gt;&gt; BTW why marcus and jorge work on Opal because the compiler is just working<br>
&gt;&gt; good?<br>
&gt;&gt;<br>
&gt;&gt; Now I will focus on 1.2, FS, Opal if I can help, and the rest but caseOf:<br>
&gt;&gt; will be in our radar.<br>
&gt;&gt;<br>
&gt;&gt; Just a remark if people would have spend the time to write mail to fix the<br>
&gt;&gt; users of caseof: in the image and<br>
&gt;&gt; other non VMmaker package we would have get already done.<br>
&gt;<br>
&gt; Fix this (rhetorical - I think this is /much/ better as a single case<br>
&gt; statement than as an index/dictionary refactoring, both in Smalltalk and<br>
&gt; when translated to C):<br>
&gt; CogIA32Compiler&gt;&gt;computeMaximumSize<br>
&gt; &quot;Compute the maximum size for each opcode.  This allows jump offsets to<br>
&gt; be determined, provided that all backward branches are long branches.&quot;<br>
&gt; &quot;N.B.  The ^maxSize := N forms are to get around the compiler&#39;s long branch<br>
&gt; limits which are exceeded when each case jumps around the otherwise.&quot;<br>
&gt; opcode caseOf: {<br>
&gt; &quot;Noops &amp; Pseudo Ops&quot;<br>
&gt; [Label] -&gt; [^maxSize := 0].<br>
&gt; [AlignmentNops] -&gt; [^maxSize := (operands at: 0) - 1].<br>
&gt; [Fill16] -&gt; [^maxSize := 2].<br>
&gt; [Fill32] -&gt; [^maxSize := 4].<br>
&gt; [FillFromWord] -&gt; [^maxSize := 4].<br>
&gt; [Nop] -&gt; [^maxSize := 1].<br>
&gt; &quot;Specific Control/Data Movement&quot;<br>
&gt; [CDQ] -&gt; [^maxSize := 1].<br>
&gt; [IDIVR] -&gt; [^maxSize := 2].<br>
&gt; [IMULRR] -&gt; [^maxSize := 3].<br>
&gt; [CPUID] -&gt; [^maxSize := 2].<br>
&gt; [CMPXCHGAwR] -&gt; [^maxSize := 7].<br>
&gt; [CMPXCHGMwrR] -&gt; [^maxSize := 8].<br>
&gt; [LFENCE] -&gt; [^maxSize := 3].<br>
&gt; [MFENCE] -&gt; [^maxSize := 3].<br>
&gt; [SFENCE] -&gt; [^maxSize := 3].<br>
&gt; [LOCK] -&gt; [^maxSize := 1].<br>
&gt; [XCHGAwR] -&gt; [^maxSize := 6].<br>
&gt; [XCHGMwrR] -&gt; [^maxSize := 7].<br>
&gt; [XCHGRR] -&gt; [^maxSize := 2].<br>
&gt; &quot;Control&quot;<br>
&gt; [Call] -&gt; [^maxSize := 5].<br>
&gt; [JumpR] -&gt; [^maxSize := 2].<br>
&gt; [Jump] -&gt; [self resolveJumpTarget. ^maxSize := 5].<br>
&gt; [JumpLong] -&gt; [self resolveJumpTarget. ^maxSize := 5].<br>
&gt; [JumpZero] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpNonZero] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpNegative] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpNonNegative] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpOverflow] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpNoOverflow] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpCarry] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpNoCarry] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpLess] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpGreaterOrEqual] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpGreater] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpLessOrEqual] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpBelow] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpAboveOrEqual] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpAbove] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpBelowOrEqual] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpLongZero] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpLongNonZero] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpFPEqual] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpFPNotEqual] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpFPLess] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpFPGreaterOrEqual] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpFPGreater] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpFPLessOrEqual] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpFPOrdered] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [JumpFPUnordered] -&gt; [self resolveJumpTarget. ^maxSize := 6].<br>
&gt; [RetN] -&gt; [^maxSize := (operands at: 0) = 0<br>
&gt; ifTrue: [1]<br>
&gt; ifFalse: [3]].<br>
&gt; &quot;Arithmetic&quot;<br>
&gt; [AddCqR] -&gt; [^maxSize := (self isQuick: (operands at: 0))<br>
&gt; ifTrue: [3]<br>
&gt; ifFalse: [(self concreteRegister: (operands at: 1)) = EAX<br>
&gt; ifTrue: [5]<br>
&gt; ifFalse: [6]]].<br>
&gt; [AndCqR] -&gt; [^maxSize := (self isQuick: (operands at: 0))<br>
&gt; ifTrue: [3]<br>
&gt; ifFalse: [(self concreteRegister: (operands at: 1)) = EAX<br>
&gt; ifTrue: [5]<br>
&gt; ifFalse: [6]]].<br>
&gt; [CmpCqR] -&gt; [^maxSize := (self isQuick: (operands at: 0))<br>
&gt; ifTrue: [3]<br>
&gt; ifFalse: [(self concreteRegister: (operands at: 1)) = EAX<br>
&gt; ifTrue: [5]<br>
&gt; ifFalse: [6]]].<br>
&gt; [OrCqR] -&gt; [^maxSize := (self isQuick: (operands at: 0))<br>
&gt; ifTrue: [3]<br>
&gt; ifFalse: [(self concreteRegister: (operands at: 1)) = EAX<br>
&gt; ifTrue: [5]<br>
&gt; ifFalse: [6]]].<br>
&gt; [SubCqR] -&gt; [^maxSize := (self isQuick: (operands at: 0))<br>
&gt; ifTrue: [3]<br>
&gt; ifFalse: [(self concreteRegister: (operands at: 1)) = EAX<br>
&gt; ifTrue: [5]<br>
&gt; ifFalse: [6]]].<br>
&gt; [AddCwR] -&gt; [^maxSize := (self concreteRegister: (operands at: 1)) = EAX<br>
&gt; ifTrue: [5]<br>
&gt; ifFalse: [6]].<br>
&gt; [CmpCwR] -&gt; [^maxSize := (self concreteRegister: (operands at: 1)) = EAX<br>
&gt; ifTrue: [5]<br>
&gt; ifFalse: [6]].<br>
&gt; [SubCwR] -&gt; [^maxSize := (self concreteRegister: (operands at: 1)) = EAX<br>
&gt; ifTrue: [5]<br>
&gt; ifFalse: [6]].<br>
&gt; [XorCwR] -&gt; [^maxSize := (self concreteRegister: (operands at: 1)) = EAX<br>
&gt; ifTrue: [5]<br>
&gt; ifFalse: [6]].<br>
&gt; [AddRR] -&gt; [^maxSize := 2].<br>
&gt; [AndRR] -&gt; [^maxSize := 2].<br>
&gt; [CmpRR] -&gt; [^maxSize := 2].<br>
&gt; [OrRR] -&gt; [^maxSize := 2].<br>
&gt; [XorRR] -&gt; [^maxSize := 2].<br>
&gt; [SubRR] -&gt; [^maxSize := 2].<br>
&gt; [NegateR] -&gt; [^maxSize := 2].<br>
&gt; [LoadEffectiveAddressMwrR]<br>
&gt; -&gt; [^maxSize := ((self isQuick: (operands at: 0))<br>
&gt; ifTrue: [3]<br>
&gt; ifFalse: [6])<br>
&gt; + ((self concreteRegister: (operands at: 1)) = ESP<br>
&gt; ifTrue: [1]<br>
&gt; ifFalse: [0])].<br>
&gt; [LogicalShiftLeftCqR] -&gt; [^maxSize := (operands at: 0) = 1<br>
&gt; ifTrue: [2]<br>
&gt; ifFalse: [3]].<br>
&gt; [LogicalShiftRightCqR] -&gt; [^maxSize := (operands at: 0) = 1<br>
&gt; ifTrue: [2]<br>
&gt; ifFalse: [3]].<br>
&gt; [ArithmeticShiftRightCqR] -&gt; [^maxSize := (operands at: 0) = 1<br>
&gt; ifTrue: [2]<br>
&gt; ifFalse: [3]].<br>
&gt; [LogicalShiftLeftRR] -&gt; [self computeShiftRRSize].<br>
&gt; [LogicalShiftRightRR] -&gt; [self computeShiftRRSize].<br>
&gt; [ArithmeticShiftRightRR] -&gt; [self computeShiftRRSize].<br>
&gt; [AddRdRd] -&gt; [^maxSize := 4].<br>
&gt; [CmpRdRd] -&gt; [^maxSize := 4].<br>
&gt; [SubRdRd] -&gt; [^maxSize := 4].<br>
&gt; [MulRdRd] -&gt; [^maxSize := 4].<br>
&gt; [DivRdRd] -&gt; [^maxSize := 4].<br>
&gt; [SqrtRd] -&gt; [^maxSize := 4].<br>
&gt; &quot;Data Movement&quot;<br>
&gt; [MoveCqR] -&gt; [^maxSize := (operands at: 0) = 0 ifTrue: [2] ifFalse: [5]].<br>
&gt; [MoveCwR] -&gt; [^maxSize := 5].<br>
&gt; [MoveRR] -&gt; [^maxSize := 2].<br>
&gt; [MoveAwR] -&gt; [^maxSize := (self concreteRegister: (operands at: 1)) = EAX<br>
&gt; ifTrue: [5]<br>
&gt; ifFalse: [6]].<br>
&gt; [MoveRAw] -&gt; [^maxSize := (self concreteRegister: (operands at: 0)) = EAX<br>
&gt; ifTrue: [5]<br>
&gt; ifFalse: [6]].<br>
&gt; [MoveRMwr] -&gt; [^maxSize := ((self isQuick: (operands at: 1))<br>
&gt; ifTrue: [3]<br>
&gt; ifFalse: [6])<br>
&gt; + ((self concreteRegister: (operands at: 2)) = ESP<br>
&gt; ifTrue: [1]<br>
&gt; ifFalse: [0])].<br>
&gt; [MoveRdM64r] -&gt; [^maxSize := ((self isQuick: (operands at: 1))<br>
&gt; ifTrue: [5]<br>
&gt; ifFalse: [8])<br>
&gt; + ((self concreteRegister: (operands at: 2)) = ESP<br>
&gt; ifTrue: [1]<br>
&gt; ifFalse: [0])].<br>
&gt; [MoveMbrR] -&gt; [^maxSize := ((self isQuick: (operands at: 0))<br>
&gt; ifTrue: [3]<br>
&gt; ifFalse: [6])<br>
&gt; + ((self concreteRegister: (operands at: 1)) = ESP<br>
&gt; ifTrue: [1]<br>
&gt; ifFalse: [0])].<br>
&gt; [MoveRMbr] -&gt; [^maxSize := ((self isQuick: (operands at: 1))<br>
&gt; ifTrue: [3]<br>
&gt; ifFalse: [6])<br>
&gt; + ((self concreteRegister: (operands at: 2)) = ESP<br>
&gt; ifTrue: [1]<br>
&gt; ifFalse: [0])].<br>
&gt; [MoveM16rR] -&gt; [^maxSize := ((self isQuick: (operands at: 0))<br>
&gt; ifTrue: [4]<br>
&gt; ifFalse: [7])<br>
&gt; + ((self concreteRegister: (operands at: 1)) = ESP<br>
&gt; ifTrue: [1]<br>
&gt; ifFalse: [0])].<br>
&gt; [MoveM64rRd] -&gt; [^maxSize := ((self isQuick: (operands at: 0))<br>
&gt; ifTrue: [5]<br>
&gt; ifFalse: [8])<br>
&gt; + ((self concreteRegister: (operands at: 1)) = ESP<br>
&gt; ifTrue: [1]<br>
&gt; ifFalse: [0])].<br>
&gt; [MoveMwrR] -&gt; [^maxSize := ((self isQuick: (operands at: 0))<br>
&gt; ifTrue: [3]<br>
&gt; ifFalse: [6])<br>
&gt; + ((self concreteRegister: (operands at: 1)) = ESP<br>
&gt; ifTrue: [1]<br>
&gt; ifFalse: [0])].<br>
&gt; [MoveXbrRR] -&gt; [self assert: (self concreteRegister: (operands at: 0)) ~=<br>
&gt; ESP.<br>
&gt; ^maxSize := (self concreteRegister: (operands at: 1)) = EBP<br>
&gt; ifTrue: [5]<br>
&gt; ifFalse: [4]].<br>
&gt; [MoveXwrRR] -&gt; [self assert: (self concreteRegister: (operands at: 0)) ~=<br>
&gt; ESP.<br>
&gt; ^maxSize := (self concreteRegister: (operands at: 1)) = EBP<br>
&gt; ifTrue: [4]<br>
&gt; ifFalse: [3]].<br>
&gt; [MoveRXwrR] -&gt; [self assert: (self concreteRegister: (operands at: 1)) ~=<br>
&gt; ESP.<br>
&gt; ^maxSize := (self concreteRegister: (operands at: 2)) = EBP<br>
&gt; ifTrue: [4]<br>
&gt; ifFalse: [3]].<br>
&gt; [PopR] -&gt; [^maxSize := 1].<br>
&gt; [PushR] -&gt; [^maxSize := 1].<br>
&gt; [PushCw] -&gt; [^maxSize := 5].<br>
&gt; [PrefetchAw] -&gt; [^maxSize := self hasSSEInstructions ifTrue: [7] ifFalse:<br>
&gt; [0]].<br>
&gt; &quot;Conversion&quot;<br>
&gt; [ConvertRRd] -&gt; [^maxSize := 4] }.<br>
&gt; ^0 &quot;to keep C compiler quiet&quot;<br>
<br>
</div></div>In which case the alternative could be to reify Instructions and let<br>
them respondsTo: maxSize, but one would still need a decoder integer<br>
-&gt; InstructionClass.<br></blockquote><div><br></div><div>That doesn&#39;t work in this context because the above has to be translated to c *without* classes.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

But if it is for the benefit of a single method, it&#39;s legitimate to<br>
wonder why bother and create so many classes.<br>
<font color="#888888"><br>
Nicolas<br>
</font><div><div></div><div class="h5"><br>
&gt;&gt;<br>
&gt;&gt; Stef<br>
&gt;&gt;<br>
&gt;&gt;<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; On Mon, Feb 14, 2011 at 11:00 PM, Stéphane Ducasse<br>
&gt;&gt; &gt; &gt; &lt;<a href="mailto:stephane.ducasse@inria.fr">stephane.ducasse@inria.fr</a>&gt; wrote:<br>
&gt;&gt; &gt; &gt;&gt;<br>
&gt;&gt; &gt; &gt;&gt; Eliot<br>
&gt;&gt; &gt; &gt;&gt;<br>
&gt;&gt; &gt; &gt;&gt; you use caseOf: for the generation of C in Slang and VM maker.<br>
&gt;&gt; &gt; &gt;&gt; Now this means that<br>
&gt;&gt; &gt; &gt;&gt;        - it does not need to be inlined<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; No.  If it is not inlined the simulator will go *much* slower.  e.g.<br>
&gt;&gt; &gt; &gt; CogVMSimulatorLSB&gt;&gt;byteAt: byteAddress<br>
&gt;&gt; &gt; &gt; | lowBits long |<br>
&gt;&gt; &gt; &gt; lowBits := byteAddress bitAnd: 3.<br>
&gt;&gt; &gt; &gt; long := self longAt: byteAddress - lowBits.<br>
&gt;&gt; &gt; &gt; ^(lowBits caseOf: {<br>
&gt;&gt; &gt; &gt; [0] -&gt; [ long ].<br>
&gt;&gt; &gt; &gt; [1] -&gt; [ long bitShift: -8  ].<br>
&gt;&gt; &gt; &gt; [2] -&gt; [ long bitShift: -16 ].<br>
&gt;&gt; &gt; &gt; [3] -&gt; [ long bitShift: -24 ]<br>
&gt;&gt; &gt; &gt; }) bitAnd: 16rFF<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; so why not put it:<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; ^ (long bitShift: (-8*lowBits) ) bitAnd: 16rFF<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; ?<br>
&gt;&gt; &gt; Or this will be slower than caseOf: ?<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; Because that was the way the code was written.  I just copied the<br>
&gt;&gt; &gt; method.  Further, it is only one example.  I&#39;m not going to rewrite the<br>
&gt;&gt; &gt; VMMaker&#39;s uses of caseOf: jyst to suit some whim of purity.  It is making<br>
&gt;&gt; &gt; unnecessary work.  Taking it out is *much* more work (/and/ emotional<br>
&gt;&gt; &gt; energy) than just leaving it alone.  Can&#39;t we try and be constructive?<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; &gt;&gt;<br>
&gt;&gt; &gt; &gt;&gt;        - it could be packaged with VMMaker<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; No.  It needs to be in the compiler to be inlined.  Why have you got<br>
&gt;&gt; &gt; &gt; on this<br>
&gt;&gt; &gt; &gt; hobby-horse?  It is a non-issue.  caseOf: ios not widelty used but<br>
&gt;&gt; &gt; &gt; extremely<br>
&gt;&gt; &gt; &gt; useful in certain circumstances.  This has the feeling of a religious<br>
&gt;&gt; &gt; &gt; pogrom, not a rational approach to the system.  IIABDFI = If It Ain&#39;t<br>
&gt;&gt; &gt; &gt; Broke,<br>
&gt;&gt; &gt; &gt; Don&#39;t Fix It.<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; This concept kinda appeal to me.<br>
&gt;&gt; &gt; From other side, i am also strongly feel that house should be kept clean<br>
&gt;&gt; &gt; :)<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; &gt;&gt;<br>
&gt;&gt; &gt; &gt;&gt; Are these two points correct?<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt; No, IMO, definitely not.<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt;&gt;<br>
&gt;&gt; &gt; &gt;&gt; Stef<br>
&gt;&gt; &gt; &gt;&gt;<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt; &gt;<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt; --<br>
&gt;&gt; &gt; Best regards,<br>
&gt;&gt; &gt; Igor Stasenko AKA sig.<br>
&gt;&gt; &gt;<br>
&gt;&gt; &gt;<br>
&gt;&gt;<br>
&gt;&gt;<br>
&gt;<br>
&gt;<br>
<br>
</div></div></blockquote></div><br>