[Vm-dev] CCodeGeneration bug or gcc compiler bug?

Nicolas Cellier nicolas.cellier.aka.nice at gmail.com
Sat Apr 6 07:59:57 UTC 2013


Yes, bad parenthesis...

#include <stdio.h>
int main()
{
  int a=0,b=11,c=22,d=33;
  int e;
  e=(a+b<c+d)?(a=4),++b:(c=6),d++;
  printf("a=%d b=%d c=%d d=%d e=%d\n",a,b,c,d,e);
  return 0;
}

prints a=4 b=12 c=22 d=34 e=12, so d++ is executed unconditionally.

while
  e=(a+b<c+d)?(a=4),++b:((c=6),d++);
prints a=4 b=12 c=22 d=33 e=12.

I will try and check if I can find a related change in COG.


2013/4/6 Eliot Miranda <eliot.miranda at gmail.com>

>
> Hi Nicolas,
>
>     I think this is bad parenthesization.  I think it's fixed in the Cog
> branch (at least I remember fixing it).  Try it in Cog.  Its late but I'll
> try and find the fix some time soon.
>
> On Fri, Apr 5, 2013 at 1:52 PM, Nicolas Cellier <
> nicolas.cellier.aka.nice at gmail.com> wrote:
>
>>
>> Let me think aloud...
>>
>> THE CONTEXT:
>>
>> backport of LargeIntegersPlugin v2.0 (32 bit native word order) to
>> Interpreter VM
>>
>> THE BUG:
>>
>> The Vm crashes with
>> http://smalltalkhub.com/#!/~nice/NiceVMExperiments/versions/VMMaker-nice.315
>>
>> THE OFFENDING METHOD:
>>
>> InterpreterPrimitives>>magnitude64BitIntegerFor: magnitude neg: isNegative
>>     "Return a Large Integer object for the given integer magnitude and
>> sign"
>>     | newLargeInteger largeClass lowWord highWord sz isSmall smallVal
>> highWord2 |
>>     <var: 'magnitude' type: #usqLong>
>>     <var: 'lowWord' type: #usqInt>
>>     <var: 'highWord' type: #usqInt>
>>     <var: 'highWord2' type: #usqInt>
>>
>>     isSmall := isNegative
>>                 ifTrue: [magnitude <= 16r40000000]
>>                 ifFalse: [magnitude < 16r40000000].
>>     isSmall ifTrue:
>>         [smallVal := self cCoerceSimple: magnitude to: #sqInt.
>>         isNegative    ifTrue: [smallVal := 0 - smallVal].
>>         ^objectMemory integerObjectOf: smallVal].
>>     largeClass := isNegative
>>         ifTrue:[self classLargeNegativeInteger]
>>         ifFalse:[self classLargePositiveInteger].
>>     lowWord := magnitude bitAnd: 16rFFFFFFFF.
>>     highWord := magnitude >> 32.
>>     highWord = 0
>>         ifTrue: [sz := 4]
>>         ifFalse:
>>             [sz := 5.
>>             (highWord2 := highWord >> 8) = 0 ifFalse: [sz := sz + 1].
>>             (highWord2 := highWord2 >> 8) = 0 ifFalse: [sz := sz + 1].
>>             (highWord2 := highWord2 >> 8) = 0 ifFalse: [sz := sz + 1].].
>>     newLargeInteger := objectMemory instantiateClass: largeClass
>> indexableSize:  sz.
>>     objectMemory storeLong32: 0 ofObject: newLargeInteger withValue:
>> lowWord.
>>     sz > 4 ifTrue: [objectMemory storeLong32: 1 ofObject: newLargeInteger
>> withValue: highWord].
>>     ^newLargeInteger
>>
>> THE CORRECTED METHOD:
>>
>> just replace the assignment
>>
>>     isNegative
>>         ifTrue:[    largeClass := self classLargeNegativeInteger]
>>         ifFalse:[    largeClass := self classLargePositiveInteger].
>>
>> THE OFFENDING GENERATED CODE:
>>
>> in src/vm/interp.c
>>
>> sqInt magnitude64BitIntegerForneg(usqLong magnitude, sqInt isNegative) {
>> register struct foo * foo = &fum;
>>     usqInt highWord;
>>     usqInt highWord2;
>>     sqInt isSmall;
>>     sqInt largeClass;
>>     usqInt lowWord;
>>     sqInt newLargeInteger;
>>     sqInt smallVal;
>>     sqInt sz;
>>     sqInt oop;
>>     sqInt oop1;
>>
>>     isSmall = (isNegative
>>         ? magnitude <= 1073741824
>>         : magnitude < 1073741824);
>>     if (isSmall) {
>>         smallVal = ((sqInt) magnitude);
>>         if (isNegative) {
>>             smallVal = 0 - smallVal;
>>         }
>>         return ((smallVal << 1) | 1);
>>     }
>>     largeClass = (isNegative
>>         ? /* begin fetchPointer:ofObject: */(oop =
>> foo->specialObjectsOop),longAt((oop + (BASE_HEADER_SIZE)) +
>> (ClassLargeNegativeInteger << (SHIFT_FOR_WORD)))
>>         : /* begin fetchPointer:ofObject: */(oop1 =
>> foo->specialObjectsOop),longAt((oop1 + (BASE_HEADER_SIZE)) +
>> (ClassLargePositiveInteger << (SHIFT_FOR_WORD))));
>>     lowWord = magnitude & 4294967295U;
>>     highWord = magnitude >> 32;
>>     if (highWord == 0) {
>>         sz = 4;
>>     } else {
>>         sz = 5;
>>         if (!(((highWord2 = ((usqInt) highWord) >> 8)) == 0)) {
>>             sz += 1;
>>         }
>>         if (!(((highWord2 = ((usqInt) highWord2) >> 8)) == 0)) {
>>             sz += 1;
>>         }
>>         if (!(((highWord2 = ((usqInt) highWord2) >> 8)) == 0)) {
>>             sz += 1;
>>         }
>>     }
>>     newLargeInteger = instantiateClassindexableSize(largeClass, sz);
>>     long32Atput((newLargeInteger + (BASE_HEADER_SIZE)) + (0 << 2),
>> lowWord);
>>     if (sz > 4) {
>>         long32Atput((newLargeInteger + (BASE_HEADER_SIZE)) + (1 << 2),
>> highWord);
>>     }
>>     return newLargeInteger;
>> }
>>
>> THE CORRECTED GENERATED CODE:
>>
>> Just change the ()?: assignment:
>>
>>     largeClass = (isNegative
>>         ? /* begin fetchPointer:ofObject: */(oop =
>> foo->specialObjectsOop),longAt((oop + (BASE_HEADER_SIZE)) +
>> (ClassLargeNegativeInteger << (SHIFT_FOR_WORD)))
>>         : /* begin fetchPointer:ofObject: */(oop1 =
>> foo->specialObjectsOop),longAt((oop1 + (BASE_HEADER_SIZE)) +
>> (ClassLargePositiveInteger << (SHIFT_FOR_WORD))));
>>
>> with an explicit if:
>>
>>     if (isNegative) {
>>         /* begin fetchPointer:ofObject: */
>>         oop = foo->specialObjectsOop;
>>         largeClass = longAt((oop + (BASE_HEADER_SIZE)) +
>> (ClassLargeNegativeInteger << (SHIFT_FOR_WORD)));
>>     } else {
>>         /* begin fetchPointer:ofObject: */
>>         oop1 = foo->specialObjectsOop;
>>         largeClass = longAt((oop1 + (BASE_HEADER_SIZE)) +
>> (ClassLargePositiveInteger << (SHIFT_FOR_WORD)));
>>     }
>>
>> THE OFFENDING METHOD JUST WORKS IN COG BUT GENERATED CODE IS DIFFERENT:
>>
>>     largeClass = (isNegative
>>         ? longAt((GIV(specialObjectsOop) + BaseHeaderSize) +
>> (ClassLargeNegativeInteger << ShiftForWord))
>>         : longAt((GIV(specialObjectsOop) + BaseHeaderSize) +
>> (ClassLargePositiveInteger << ShiftForWord)));
>>
>> THE QUESTION:
>>
>> I just don't understand what is incorrect in the (t)?a,b:c,d; construct...
>> Could it be that it is interpreted:
>>
>>     ((t)?a,b:c),d;
>>
>> Ah if so, maybe that would explain the warning about oop1 being
>> potentially used before assigned! (a bulb just enlighten)
>>
>> In this case we have a bug in the generator (probably a known one since
>> we rearely ever use x := test ifTrue: [] ifFalse: [] in Slang)...
>>
>> Nicolas
>>
>>
>>
>>
>>
>>
>
>
> --
> best,
> Eliot
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20130406/62562ec5/attachment-0001.htm


More information about the Vm-dev mailing list