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

Benoit St-Jean bstjean at yahoo.com
Tue Feb 15 22:28:57 CET 2011


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...

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.
>>
>>
>
>
>


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.gforge.inria.fr/pipermail/pharo-project/attachments/20110215/271f1887/attachment.htm>


More information about the Pharo-project mailing list