about the usage of bitShift: for directed shifts in VMMaker

Hi again,

I see this horrible generated code:

    /* Spur64BitMemoryManager>>#integerValueOf: */
sqInt
integerValueOf(sqInt oop)
{
    return ((((usqInt) oop >> 63)) == 1
        ? ((((((-(numTagBits())) < 0) ? ((usqInt) oop >> -(-(numTagBits()))) : ((usqInt) oop << (-(numTagBits()))))) & 0x1FFFFFFFFFFFFFFFLL) - 0x1FFFFFFFFFFFFFFFLL) - 1
        : (((-(numTagBits())) < 0) ? ((usqInt) oop >> -(-(numTagBits()))) : ((usqInt) oop << (-(numTagBits())))));
}

which of course generates a (false positive) warning:

Avertissement    C4293    '<<' : compteur de décalage négatif ou trop important, comportement non défini    SqueakCogSpur    X:\Smalltalk\opensmalltalk-vm\spur64src\vm\cointerp.c    41564   

Slang is:

integerValueOf: oop
    "Translator produces 'oop >> 3'"
    ^(oop bitShift: -63) = 1 "tests top bit"
        ifTrue: "negative"
            [((oop bitShift: self numTagBits negated) bitAnd: 16r1FFFFFFFFFFFFFFF) - 16r1FFFFFFFFFFFFFFF - 1  "Faster than -16r4000000000000000 (a LgInt)"]
        ifFalse: "positive"
            [oop bitShift: self numTagBits negated]

We see that the constant -63 has been translated into a directed (right) shift, but the other constant (numTagBits) not so, either because constant substitution is not aggressive enough, or because negated made an expression of the constant...

However, the intention is ALWAYS to perform a directed shift (to the right) so the code could be expressed to make both intention AND generated code clear (and warning-free):

integerValueOf: oop
    "Translator produces 'oop >> 3'"
    ^(oop >> 63) = 1 "tests top bit"
        ifTrue: "negative"
            [((oop >> self numTagBits) bitAnd: 16r1FFFFFFFFFFFFFFF) - 16r1FFFFFFFFFFFFFFF - 1  "Faster than -16r4000000000000000 (a LgInt)"]
        ifFalse: "positive"
            [oop >> self numTagBits]

Why use bitShift: then? Is it a problem of speed for the VM simulator?