On Tue, Feb 15, 2011 at 11:17 PM, Stéphane Ducasse < stephane.ducasse@inria.fr> wrote:
But it looks like a DSL to me.
No its not. caseOf: is valid Smalltalk. It is another control structure defined in the library rather than by the language, just like do:, inject:into: et al. It is extremely useful in certain circumstances. It can be (and is) optimized. Functional languages support case statements that are conceptually similar. caseOf: (and those of functional languages) are *much* more powerful than the switch statement of C: caseOf: can dispatch on arbitrary values, not just integer indices; caseOf:'s selectors (the things on the left of the ->'s) can be expressions, not just constants.
So caseOf: could be moved to Cog.
Fine. Let me be equally pig-headed then. I'm not going to spend any more energy on this, and I'm not going to spend any more energy on Cog in Pharo. This is ridiculous.
Stef
On Feb 15, 2011, at 10:28 PM, Benoit St-Jean 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...
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@gmail.com> To: Pharo-project@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@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@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.