[Pharo-project] could we agree to remove caseOf: and caseOf:otherwise:

Igor Stasenko siguctua at gmail.com
Wed Feb 16 00:54:10 CET 2011


On 15 February 2011 22:28, Benoit St-Jean <bstjean at yahoo.com> wrote:
> Eliot,
>
> Now I fully understand the pain you'd have to go through (and others
> involved in the VM) if we'd remove this method we don't "like"...  I guess
> it's way easier to live with #caseOf: than live without it...
>
yeah, in smalltalk you can just reify it to a lot of classes (one for
each instruction) and implement #computeMaximumSize per each.
Oh.. and you even don't have to do that per each.. if you look at
code, it has size=6 for jumps.
So, you can implement only 1 method in base 'jump' class, while others
will simply reuse it.


> CogIA32Compiler>>#computeMaximumSize, when one method is worth a thousand
> words...
>
>
>
> -----------------
> Benoit St-Jean
> Yahoo! Messenger: bstjean
> A standpoint is an intellectual horizon of radius zero.
> (Albert Einstein)
>
> ________________________________
> From: Eliot Miranda <eliot.miranda at gmail.com>
> To: Pharo-project at lists.gforge.inria.fr
> Sent: Tue, February 15, 2011 3:56:11 PM
> Subject: Re: [Pharo-project] could we agree to remove caseOf: and
> caseOf:otherwise:
>
>
>
> On Tue, Feb 15, 2011 at 12:39 PM, Stéphane Ducasse
> <stephane.ducasse at inria.fr> wrote:
>>
>> Eliot
>>
>> I can understand that well. Now we could just let this code in VMMaker and
>> not inlining.
>> We fix 8 users and we are done. I have concerned that we have a complex
>> solution because it is complex
>> for a couple of use case. Of course we can do nothing (and we will
>> probably not do it now) but in that case we can
>> also stop pharo right now because a large part of the system could follow
>> the exact same principle.
>
> That's a false distinction.  There's nothing wrong with caseOf: and not
> addressing it will not leave Pharo any the worse.  Case statements are not
> in themselves evil in the right context.  The main problems with the
> index/dictionary approach are that
> a) the solution is no longer contained in a single method
> b) the dictionary is metadata that needs to be updated in an initialize
> method; forget to initialize the dictionary when an index changes and your
> program is broken.
> As my first wife said "don't sweat the petty stuff; pet the sweaty stuff".
>
> BTW, there's a false assumption that has been stated in this thread that the
> caseOf: statement is only used with explicit integers.  It can be used with
> anything.  Expressions, class constants etc.
>>
>> BTW why marcus and jorge work on Opal because the compiler is just working
>> good?
>>
>> Now I will focus on 1.2, FS, Opal if I can help, and the rest but caseOf:
>> will be in our radar.
>>
>> Just a remark if people would have spend the time to write mail to fix the
>> users of caseof: in the image and
>> other non VMmaker package we would have get already done.
>
> Fix this (rhetorical - I think this is /much/ better as a single case
> statement than as an index/dictionary refactoring, both in Smalltalk and
> when translated to C):
> CogIA32Compiler>>computeMaximumSize
> "Compute the maximum size for each opcode.  This allows jump offsets to
> be determined, provided that all backward branches are long branches."
> "N.B.  The ^maxSize := N forms are to get around the compiler's long branch
> limits which are exceeded when each case jumps around the otherwise."
> opcode caseOf: {
> "Noops & Pseudo Ops"
> [Label] -> [^maxSize := 0].
> [AlignmentNops] -> [^maxSize := (operands at: 0) - 1].
> [Fill16] -> [^maxSize := 2].
> [Fill32] -> [^maxSize := 4].
> [FillFromWord] -> [^maxSize := 4].
> [Nop] -> [^maxSize := 1].
> "Specific Control/Data Movement"
> [CDQ] -> [^maxSize := 1].
> [IDIVR] -> [^maxSize := 2].
> [IMULRR] -> [^maxSize := 3].
> [CPUID] -> [^maxSize := 2].
> [CMPXCHGAwR] -> [^maxSize := 7].
> [CMPXCHGMwrR] -> [^maxSize := 8].
> [LFENCE] -> [^maxSize := 3].
> [MFENCE] -> [^maxSize := 3].
> [SFENCE] -> [^maxSize := 3].
> [LOCK] -> [^maxSize := 1].
> [XCHGAwR] -> [^maxSize := 6].
> [XCHGMwrR] -> [^maxSize := 7].
> [XCHGRR] -> [^maxSize := 2].
> "Control"
> [Call] -> [^maxSize := 5].
> [JumpR] -> [^maxSize := 2].
> [Jump] -> [self resolveJumpTarget. ^maxSize := 5].
> [JumpLong] -> [self resolveJumpTarget. ^maxSize := 5].
> [JumpZero] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpNonZero] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpNegative] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpNonNegative] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpOverflow] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpNoOverflow] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpCarry] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpNoCarry] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpLess] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpGreaterOrEqual] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpGreater] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpLessOrEqual] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpBelow] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpAboveOrEqual] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpAbove] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpBelowOrEqual] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpLongZero] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpLongNonZero] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpFPEqual] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpFPNotEqual] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpFPLess] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpFPGreaterOrEqual] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpFPGreater] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpFPLessOrEqual] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpFPOrdered] -> [self resolveJumpTarget. ^maxSize := 6].
> [JumpFPUnordered] -> [self resolveJumpTarget. ^maxSize := 6].
> [RetN] -> [^maxSize := (operands at: 0) = 0
> ifTrue: [1]
> ifFalse: [3]].
> "Arithmetic"
> [AddCqR] -> [^maxSize := (self isQuick: (operands at: 0))
> ifTrue: [3]
> ifFalse: [(self concreteRegister: (operands at: 1)) = EAX
> ifTrue: [5]
> ifFalse: [6]]].
> [AndCqR] -> [^maxSize := (self isQuick: (operands at: 0))
> ifTrue: [3]
> ifFalse: [(self concreteRegister: (operands at: 1)) = EAX
> ifTrue: [5]
> ifFalse: [6]]].
> [CmpCqR] -> [^maxSize := (self isQuick: (operands at: 0))
> ifTrue: [3]
> ifFalse: [(self concreteRegister: (operands at: 1)) = EAX
> ifTrue: [5]
> ifFalse: [6]]].
> [OrCqR] -> [^maxSize := (self isQuick: (operands at: 0))
> ifTrue: [3]
> ifFalse: [(self concreteRegister: (operands at: 1)) = EAX
> ifTrue: [5]
> ifFalse: [6]]].
> [SubCqR] -> [^maxSize := (self isQuick: (operands at: 0))
> ifTrue: [3]
> ifFalse: [(self concreteRegister: (operands at: 1)) = EAX
> ifTrue: [5]
> ifFalse: [6]]].
> [AddCwR] -> [^maxSize := (self concreteRegister: (operands at: 1)) = EAX
> ifTrue: [5]
> ifFalse: [6]].
> [CmpCwR] -> [^maxSize := (self concreteRegister: (operands at: 1)) = EAX
> ifTrue: [5]
> ifFalse: [6]].
> [SubCwR] -> [^maxSize := (self concreteRegister: (operands at: 1)) = EAX
> ifTrue: [5]
> ifFalse: [6]].
> [XorCwR] -> [^maxSize := (self concreteRegister: (operands at: 1)) = EAX
> ifTrue: [5]
> ifFalse: [6]].
> [AddRR] -> [^maxSize := 2].
> [AndRR] -> [^maxSize := 2].
> [CmpRR] -> [^maxSize := 2].
> [OrRR] -> [^maxSize := 2].
> [XorRR] -> [^maxSize := 2].
> [SubRR] -> [^maxSize := 2].
> [NegateR] -> [^maxSize := 2].
> [LoadEffectiveAddressMwrR]
> -> [^maxSize := ((self isQuick: (operands at: 0))
> ifTrue: [3]
> ifFalse: [6])
> + ((self concreteRegister: (operands at: 1)) = ESP
> ifTrue: [1]
> ifFalse: [0])].
> [LogicalShiftLeftCqR] -> [^maxSize := (operands at: 0) = 1
> ifTrue: [2]
> ifFalse: [3]].
> [LogicalShiftRightCqR] -> [^maxSize := (operands at: 0) = 1
> ifTrue: [2]
> ifFalse: [3]].
> [ArithmeticShiftRightCqR] -> [^maxSize := (operands at: 0) = 1
> ifTrue: [2]
> ifFalse: [3]].
> [LogicalShiftLeftRR] -> [self computeShiftRRSize].
> [LogicalShiftRightRR] -> [self computeShiftRRSize].
> [ArithmeticShiftRightRR] -> [self computeShiftRRSize].
> [AddRdRd] -> [^maxSize := 4].
> [CmpRdRd] -> [^maxSize := 4].
> [SubRdRd] -> [^maxSize := 4].
> [MulRdRd] -> [^maxSize := 4].
> [DivRdRd] -> [^maxSize := 4].
> [SqrtRd] -> [^maxSize := 4].
> "Data Movement"
> [MoveCqR] -> [^maxSize := (operands at: 0) = 0 ifTrue: [2] ifFalse: [5]].
> [MoveCwR] -> [^maxSize := 5].
> [MoveRR] -> [^maxSize := 2].
> [MoveAwR] -> [^maxSize := (self concreteRegister: (operands at: 1)) = EAX
> ifTrue: [5]
> ifFalse: [6]].
> [MoveRAw] -> [^maxSize := (self concreteRegister: (operands at: 0)) = EAX
> ifTrue: [5]
> ifFalse: [6]].
> [MoveRMwr] -> [^maxSize := ((self isQuick: (operands at: 1))
> ifTrue: [3]
> ifFalse: [6])
> + ((self concreteRegister: (operands at: 2)) = ESP
> ifTrue: [1]
> ifFalse: [0])].
> [MoveRdM64r] -> [^maxSize := ((self isQuick: (operands at: 1))
> ifTrue: [5]
> ifFalse: [8])
> + ((self concreteRegister: (operands at: 2)) = ESP
> ifTrue: [1]
> ifFalse: [0])].
> [MoveMbrR] -> [^maxSize := ((self isQuick: (operands at: 0))
> ifTrue: [3]
> ifFalse: [6])
> + ((self concreteRegister: (operands at: 1)) = ESP
> ifTrue: [1]
> ifFalse: [0])].
> [MoveRMbr] -> [^maxSize := ((self isQuick: (operands at: 1))
> ifTrue: [3]
> ifFalse: [6])
> + ((self concreteRegister: (operands at: 2)) = ESP
> ifTrue: [1]
> ifFalse: [0])].
> [MoveM16rR] -> [^maxSize := ((self isQuick: (operands at: 0))
> ifTrue: [4]
> ifFalse: [7])
> + ((self concreteRegister: (operands at: 1)) = ESP
> ifTrue: [1]
> ifFalse: [0])].
> [MoveM64rRd] -> [^maxSize := ((self isQuick: (operands at: 0))
> ifTrue: [5]
> ifFalse: [8])
> + ((self concreteRegister: (operands at: 1)) = ESP
> ifTrue: [1]
> ifFalse: [0])].
> [MoveMwrR] -> [^maxSize := ((self isQuick: (operands at: 0))
> ifTrue: [3]
> ifFalse: [6])
> + ((self concreteRegister: (operands at: 1)) = ESP
> ifTrue: [1]
> ifFalse: [0])].
> [MoveXbrRR] -> [self assert: (self concreteRegister: (operands at: 0)) ~=
> ESP.
> ^maxSize := (self concreteRegister: (operands at: 1)) = EBP
> ifTrue: [5]
> ifFalse: [4]].
> [MoveXwrRR] -> [self assert: (self concreteRegister: (operands at: 0)) ~=
> ESP.
> ^maxSize := (self concreteRegister: (operands at: 1)) = EBP
> ifTrue: [4]
> ifFalse: [3]].
> [MoveRXwrR] -> [self assert: (self concreteRegister: (operands at: 1)) ~=
> ESP.
> ^maxSize := (self concreteRegister: (operands at: 2)) = EBP
> ifTrue: [4]
> ifFalse: [3]].
> [PopR] -> [^maxSize := 1].
> [PushR] -> [^maxSize := 1].
> [PushCw] -> [^maxSize := 5].
> [PrefetchAw] -> [^maxSize := self hasSSEInstructions ifTrue: [7] ifFalse:
> [0]].
> "Conversion"
> [ConvertRRd] -> [^maxSize := 4] }.
> ^0 "to keep C compiler quiet"
>>
>> Stef
>>
>>
>> > >
>> > >
>> > > On Mon, Feb 14, 2011 at 11:00 PM, Stéphane Ducasse
>> > > <stephane.ducasse at inria.fr> wrote:
>> > >>
>> > >> Eliot
>> > >>
>> > >> you use caseOf: for the generation of C in Slang and VM maker.
>> > >> Now this means that
>> > >>        - it does not need to be inlined
>> > >
>> > > No.  If it is not inlined the simulator will go *much* slower.  e.g.
>> > > CogVMSimulatorLSB>>byteAt: byteAddress
>> > > | lowBits long |
>> > > lowBits := byteAddress bitAnd: 3.
>> > > long := self longAt: byteAddress - lowBits.
>> > > ^(lowBits caseOf: {
>> > > [0] -> [ long ].
>> > > [1] -> [ long bitShift: -8  ].
>> > > [2] -> [ long bitShift: -16 ].
>> > > [3] -> [ long bitShift: -24 ]
>> > > }) bitAnd: 16rFF
>> > >
>> >
>> > so why not put it:
>> >
>> > ^ (long bitShift: (-8*lowBits) ) bitAnd: 16rFF
>> >
>> > ?
>> > Or this will be slower than caseOf: ?
>> >
>> > Because that was the way the code was written.  I just copied the
>> > method.  Further, it is only one example.  I'm not going to rewrite the
>> > VMMaker's uses of caseOf: jyst to suit some whim of purity.  It is making
>> > unnecessary work.  Taking it out is *much* more work (/and/ emotional
>> > energy) than just leaving it alone.  Can't we try and be constructive?
>> >
>> >
>> >
>> > >>
>> > >>        - it could be packaged with VMMaker
>> > >
>> > > No.  It needs to be in the compiler to be inlined.  Why have you got
>> > > on this
>> > > hobby-horse?  It is a non-issue.  caseOf: ios not widelty used but
>> > > extremely
>> > > useful in certain circumstances.  This has the feeling of a religious
>> > > pogrom, not a rational approach to the system.  IIABDFI = If It Ain't
>> > > Broke,
>> > > Don't Fix It.
>> >
>> > This concept kinda appeal to me.
>> > From other side, i am also strongly feel that house should be kept clean
>> > :)
>> >
>> > >>
>> > >> Are these two points correct?
>> > >
>> > > No, IMO, definitely not.
>> > >
>> > >>
>> > >> Stef
>> > >>
>> > >
>> > >
>> >
>> >
>> >
>> > --
>> > Best regards,
>> > Igor Stasenko AKA sig.
>> >
>> >
>>
>>
>
>
>



-- 
Best regards,
Igor Stasenko AKA sig.




More information about the Pharo-project mailing list