<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body dir="auto"><div>Hi Nicolas,<br><br></div><div><br>On Sep 3, 2017, at 6:38 AM, Nicolas Cellier <<a href="mailto:nicolas.cellier.aka.nice@gmail.com">nicolas.cellier.aka.nice@gmail.com</a>> wrote:<br><br></div><blockquote type="cite"><div><span></span></div></blockquote><blockquote type="cite"><div><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2017-08-31 21:21 GMT+02:00  <span dir="ltr"><<a href="mailto:commits@source.squeak.org" target="_blank">commits@source.squeak.org</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
Eliot Miranda uploaded a new version of VMMaker to project VM Maker:<br>
<a href="http://source.squeak.org/VMMaker/VMMaker.oscog-eem.2266.mcz" rel="noreferrer" target="_blank">http://source.squeak.org/VMMak<wbr>er/VMMaker.oscog-eem.2266.mcz</a><br>
<br>
==================== Summary ====================<br>
<br>
Name: VMMaker.oscog-eem.2266<br>
Author: eem<br>
Time: 31 August 2017, 12:20:30.681037 pm<br>
UUID: 0501d71a-3185-4bdb-a99c-76a5fb<wbr>beee22<br>
Ancestors: VMMaker.oscog-eem.2265<br>
<br>
Spur:<br>
Simplify cleverSwapHeaders:and:copyHash<wbr>Flag: via xor.<br>
Fix the type of currentAllocatedBytes given the Slang changes below.<br>
<br>
Slang:<br>
Fix a regression caused by the Slang fixes in VMMaker.oscog-eem.2243.<br>
Since inferTypesForImplicitlyTypedVa<wbr>riablesIn: no longer sets the types of locals as it goes along (which was incorrect) we can no longer default the types of untyped variables to sqInt for the purposes of returnTypeForSend:in:ifNil:.  To allow returnTypeForSend:in:ifNil: to answer nil for arithmetic on untyped expressions typeForArithmetic:in: uses TParseNode>>typeOrNilFrom:in: instead of CCodeGenerator>>typeFor:in: to avoid the defaulting.  returnTypeForSend:in:ifNil: has been refactored to use the more direct TParseNode>>typeFrom:in: instead of CCodeGenerator>>typeFor:in: for clarity.<br>
Teh regression caused Slang to fail to infer the types of remembered1/2 & hash1/2 in cleverSwapHeaders:and:copyHash<wbr>Flag:.  The new code correctly infers the types of remembered & hashBits as sqLong.<br>
<br></blockquote><div><br></div><div>OK, the differences seem a bit germane at first glance, but I can try to understand better:<br></div><div><br></div><div>So CCodeGenerator>>typeFor:in: / TParseNode>>typeFrom:in: do default to #sqInt rather than nil,.</div><div>This did cause a premature typing of some local variables.</div><div>So it was better to use typeOrNilFrom:in:in some place.<br></div><div><br></div><div>But If we forget to replace some occurrence by typeOrNilFrom:in: ,</div><div>then we might chain a type inference returning nil with with a type inference returning default #sqInt value,</div><div>and thus we will bypass the correct type inferencing.</div><div>Was it the scenario of the problem?<br></div></div></div></div></div></blockquote><div><br></div>Yes.  (Sorry, I missed your message)<div><br><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
SoundPlugin:<br>
Eliminate a couple of warnings by using 0 instead of NULL.<br>
<br>
=============== Diff against VMMaker.oscog-eem.2265 ===============<br>
<br>
Item was changed:<br>
  ----- Method: CCodeGenerator>>returnTypeForS<wbr>end:in:ifNil: (in category 'type inference') -----<br>
  returnTypeForSend: sendNode in: aTMethod ifNil: typeIfNil<br>
        "Answer the return type for a send.  Unbound sends default to typeIfNil.<br>
         Methods with types as yet unknown have a type determined either by the<br>
         kernelReturnTypes or the table below, or, if they are in neither set, then nil.<br>
         The inferred type should match as closely as possible the C type of<br>
         generated expessions so that inlining would not change the expression.<br>
         If there is a method for sel but its return type is as yet unknown it mustn't<br>
         be defaulted, since on a subsequent pass its type may be computable."<br>
        | sel methodOrNil |<br>
        methodOrNil := self anyMethodNamed: (sel := sendNode selector).<br>
        (methodOrNil notNil and: [methodOrNil returnType notNil]) ifTrue:<br>
                [^self baseTypeForType: methodOrNil returnType].<br>
        ^kernelReturnTypes<br>
                at: sel<br>
                ifAbsent:<br>
                        [sel<br>
                                caseOf: {<br>
                                [#integerValueOf:]              ->      [#sqInt].<br>
                                [#isIntegerObject:]             ->      [#int].<br>
+                               [#negated]                              ->      [self promoteArithmeticTypes: (sendNode receiver typeFrom: self in: aTMethod) and: #int].<br>
-                               [#negated]                              ->      [self promoteArithmeticTypes: (self typeFor: sendNode receiver in: aTMethod) and: #int].<br>
                                [#+]                                    ->      [self typeForArithmetic: sendNode in: aTMethod].<br>
                                [#-]                                            ->      [self typeForArithmetic: sendNode in: aTMethod].<br>
                                [#*]                                    ->      [self typeForArithmetic: sendNode in: aTMethod].<br>
                                [#/]                                            ->      [self typeForArithmetic: sendNode in: aTMethod].<br>
                                [#//]                                   ->      [self typeForArithmetic: sendNode in: aTMethod].<br>
                                [#\\]                                   ->      [self typeForArithmetic: sendNode in: aTMethod].<br>
                                [#rem:]                                 ->      [self typeForArithmetic: sendNode in: aTMethod].<br>
                                [#quo:]                                 ->      [self typeForArithmetic: sendNode in: aTMethod].<br>
                                "C99 Sec Bitwise shift operators ... 3 Sematics ...<br>
                                 The integer promotions are performed on each of the operands. The type of the result is that of the promoted left operand..."<br>
+                               [#>>]                                   ->      [sendNode receiver typeFrom: self in: aTMethod].<br>
+                               [#<<]                                   ->      [sendNode receiver typeFrom: self in: aTMethod].<br>
+                               [#addressOf:]                   ->      [(sendNode receiver typeFrom: self in: aTMethod)<br>
-                               [#>>]                                   ->      [self typeFor: sendNode receiver in: aTMethod].<br>
-                               [#<<]                                   ->      [self typeFor: sendNode receiver in: aTMethod].<br>
-                               [#addressOf:]                   ->      [(self typeFor: sendNode receiver in: aTMethod)<br>
                                                                                                ifNil: [#sqInt]<br>
                                                                                                ifNotNil: [:type| type, (type last isLetter ifTrue: [' *'] ifFalse: ['*'])]].<br>
                                [#at:]                                  ->      [self typeForDereference: sendNode in: aTMethod].<br>
                                [#bitAnd:]                              ->      [self typeForArithmetic: sendNode in: aTMethod].<br>
                                [#bitOr:]                               ->      [self typeForArithmetic: sendNode in: aTMethod].<br>
                                [#bitXor:]                              ->      [self typeForArithmetic: sendNode in: aTMethod].<br>
                                [#bitClear:]                            ->      [self typeForArithmetic: sendNode in: aTMethod].<br>
                                [#bitInvert32]                  ->      [#'unsigned int'].<br>
+                               [#bitInvert64]                  ->      [self promoteArithmeticTypes: (sendNode receiver typeFrom: self in: aTMethod) and: #int].<br>
-                               [#bitInvert64]                  ->      [self promoteArithmeticTypes: (self typeFor: sendNode receiver in: aTMethod) and: #int].<br>
                                [#byteSwap32]                   ->      [#'unsigned int'].<br>
                                [#byteSwap64]                   ->      [#'unsigned long long'].<br>
                                [#byteSwapped32IfBigEndian:]    ->      [#'unsigned int'].<br>
                                [#byteSwapped64IfBigEndian:]    ->      [#'unsigned long long'].<br>
                                [#=]                                    ->      [#int].<br>
                                [#~=]                                   ->      [#int].<br>
                                [#==]                                   ->      [#int].<br>
                                [#~~]                                   ->      [#int].<br>
                                [#<]                                    ->      [#int].<br>
                                [#<=]                                   ->      [#int].<br>
                                [#>]                                    ->      [#int].<br>
                                [#>=]                                   ->      [#int].<br>
                                [#between:and:]         ->      [#int].<br>
                                [#anyMask:]                             ->      [#int].<br>
                                [#allMask:]                             ->      [#int].<br>
                                [#noMask:]                              ->      [#int].<br>
                                [#isNil]                                        ->      [#int].<br>
                                [#notNil]                               ->      [#int].<br>
                                [#&]                                    ->      [#int].<br>
                                [#|]                                            ->      [#int].<br>
                                [#not]                                  ->      [#int].<br>
                                [#asFloat]                              ->      [#double].<br>
                                [#atan]                                 ->      [#double].<br>
                                [#exp]                                  ->      [#double].<br>
                                [#log]                                  ->      [#double].<br>
                                [#sin]                                  ->      [#double].<br>
                                [#sqrt]                                 ->      [#double].<br>
                                [#asLong]                               ->      [#long].<br>
                                [#asInteger]                    ->      [#sqInt].<br>
                                [#asIntegerPtr]                 ->      [#'sqIntptr_t'].<br>
                                [#asUnsignedInteger]    ->      [#usqInt].<br>
                                [#asUnsignedIntegerPtr]->       [#'usqIntptr_t'].<br>
                                [#asUnsignedLong]               ->      [#'unsigned long'].<br>
                                [#asUnsignedLongLong]           ->      [#'unsigned long long'].<br>
                                [#asVoidPointer]                ->      [#'void *'].<br>
                                [#signedIntToLong]              ->      [#usqInt]. "c.f. generateSignedIntToLong:on:ind<wbr>ent:"<br>
                                [#signedIntToShort]     ->      [#usqInt]. "c.f. generateSignedIntToShort:on:in<wbr>dent:"<br>
                                [#cCoerce:to:]                  ->      [sendNode args last value].<br>
                                [#cCoerceSimple:to:]    ->      [sendNode args last value].<br>
                                [#sizeof:]                              ->      [#'usqIntptr_t']. "Technically it's a size_t but it matches on target architectures so far..."<br>
                                [#ifTrue:ifFalse:]              ->      [self typeForConditional: sendNode in: aTMethod].<br>
                                [#ifFalse:ifTrue:]              ->      [self typeForConditional: sendNode in: aTMethod].<br>
                                [#ifTrue:]                              ->      [self typeForConditional: sendNode in: aTMethod].<br>
                                [#ifFalse:]                             ->      [self typeForConditional: sendNode in: aTMethod].<br>
                                [#and:]                                 ->      [#sqInt].<br>
                                [#or:]                                  ->      [#sqInt].<br>
                                [#caseOf:]                              ->      [self typeFor: sendNode args first in: aTMethod] }<br>
                                otherwise: "If there /is/ a method for sel but its return type is as yet unknown it /mustn't/ be defaulted,<br>
                                                        since on a subsequent pass its type may be computable.  Only default unbound selectors."<br>
                                        [methodOrNil ifNotNil: [nil] ifNil: [typeIfNil]]]!<br>
<br>
Item was changed:<br>
  ----- Method: CCodeGenerator>>typeForArithme<wbr>tic:in: (in category 'type inference') -----<br>
  typeForArithmetic: sendNode in: aTMethod<br>
        "Answer the return type for an arithmetic sendThis is so that the inliner can still<br>
         inline simple expressions.  Deal with pointer arithmetic, floating point arithmetic<br>
         and promotion."<br>
+       | rcvrType argType arg |<br>
+       rcvrType := sendNode receiver typeOrNilFrom: self in: aTMethod.<br>
+       argType := (arg := sendNode args first) typeOrNilFrom: self in: aTMethod.<br>
-       | rcvrType argType arg promotedType |<br>
-       rcvrType := self typeFor: sendNode receiver in: aTMethod.<br>
-       argType := self typeFor: (arg := sendNode args first) in: aTMethod.<br>
        "deal with pointer arithmetic"<br>
        ((rcvrType notNil and: [rcvrType last == $*]) or: [argType notNil and: [argType last == $*]]) ifTrue:<br>
                [(rcvrType isNil or: [argType isNil]) ifTrue:<br>
                        [^nil].<br>
                 (rcvrType last == $* and: [argType last == $*]) ifTrue:<br>
                        [sendNode selector == #- ifTrue:<br>
                                [^#int].<br>
                         self error: 'invalid pointer arithmetic'].<br>
                 ^rcvrType last == $*<br>
                        ifTrue: [rcvrType]<br>
                        ifFalse: [argType]].<br>
+       ^(self promoteArithmeticTypes: rcvrType and: argType) ifNotNil:<br>
+               [:promotedType|<br>
+                "We have to be very careful with subtraction.  The difference between two unsigned types is signed.<br>
+                 But we don't want unsigned - constant to be signed.  We almost always want this to stay unsigned."<br>
+                (sendNode selector == #- and: [promotedType first == $u and: [(arg isConstant and: [arg value isInteger]) not]])<br>
+                       ifTrue: [promotedType allButFirst: ((promotedType beginsWith: 'unsigned') ifTrue: [9] ifFalse: [1])]<br>
+                       ifFalse: [promotedType]]!<br>
-       promotedType := self promoteArithmeticTypes: rcvrType and: argType.<br>
-       "We have to be very careful with subtraction.  The difference between two unsigned types is signed.<br>
-        But we don't want unsigned - constant to be signed.  We almost always want this to stay unsigned."<br>
-       ^(sendNode selector == #- and: [promotedType first == $u and: [(arg isConstant and: [arg value isInteger]) not]])<br>
-               ifTrue: [promotedType allButFirst: ((promotedType beginsWith: 'unsigned') ifTrue: [9] ifFalse: [1])]<br>
-               ifFalse: [promotedType]!<br>
<br>
Item was changed:<br>
  ----- Method: SoundPlugin>>primitiveSetDefau<wbr>ltSoundPlayer (in category 'primitives') -----<br>
  primitiveSetDefaultSoundPlayer<br>
        "Tell the operating system to use the specified device name as the output device for sound."<br>
        "arg at top of stack is the String"<br>
        | deviceName obj srcPtr sz |<br>
        <export: true><br>
        <var: 'deviceName' declareC: 'char deviceName[257]'><br>
        <var: 'srcPtr' type: #'char *'><br>
<br>
        "Parse arguments"<br>
        interpreterProxy methodArgumentCount = 1 ifFalse:<br>
                [^interpreterProxy primitiveFail].<br>
        obj := interpreterProxy stackValue: 0.<br>
        (interpreterProxy isBytes: obj) ifFalse:<br>
                [^interpreterProxy primitiveFail].<br>
        (sz := interpreterProxy byteSizeOf: obj) <= 256 ifFalse:<br>
                [^interpreterProxy primitiveFail].<br>
        srcPtr := interpreterProxy firstIndexableField: obj.<br>
        self touch: srcPtr.<br>
        self touch: deviceName.<br>
        self touch: sz.<br>
        self cCode: 'strncpy(deviceName, srcPtr, sz)'.<br>
+       self cCode: 'deviceName[sz] = 0'.<br>
-       self cCode: 'deviceName[sz] = NULL'.<br>
<br>
        "do the work"<br>
        self cCode: 'setDefaultSoundPlayer(deviceN<wbr>ame)'.<br>
        interpreterProxy failed ifFalse: "pop arg, leave receiver"<br>
                [interpreterProxy pop: 1]!<br>
<br>
Item was changed:<br>
  ----- Method: SoundPlugin>>primitiveSetDefau<wbr>ltSoundRecorder (in category 'primitives') -----<br>
  primitiveSetDefaultSoundRecord<wbr>er<br>
        "Tell the operating system to use the specified device name as the input device for sound."<br>
        "arg at top of stack is the String"<br>
        | deviceName obj srcPtr sz |<br>
        <export: true><br>
        <var: 'deviceName' declareC: 'char deviceName[257]'><br>
        <var: 'srcPtr' type: #'char *'><br>
<br>
        "Parse arguments"<br>
        interpreterProxy methodArgumentCount = 1 ifFalse:<br>
                [^interpreterProxy primitiveFail].<br>
        obj := interpreterProxy stackValue: 0.<br>
        (interpreterProxy isBytes: obj) ifFalse:<br>
                [^interpreterProxy primitiveFail].<br>
        (sz := interpreterProxy byteSizeOf: obj) <= 256 ifFalse:<br>
                [^interpreterProxy primitiveFail].<br>
        srcPtr := interpreterProxy firstIndexableField: obj.<br>
        self touch: srcPtr.<br>
        self touch: deviceName.<br>
        self touch: sz.<br>
        self cCode: 'strncpy(deviceName, srcPtr, sz)'.<br>
+       self cCode: 'deviceName[sz] = 0'.<br>
-       self cCode: 'deviceName[sz] = NULL'.<br>
<br>
        "do the work"<br>
        self cCode: 'setDefaultSoundRecorder(devic<wbr>eName)'.<br>
        interpreterProxy failed ifFalse: "pop arg, leave receiver"<br>
                [interpreterProxy pop: 1]!<br>
<br>
Item was changed:<br>
  ----- Method: SpurMemoryManager>>cleverSwapH<wbr>eaders:and:copyHashFlag: (in category 'become implementation') -----<br>
  cleverSwapHeaders: obj1 and: obj2 copyHashFlag: copyHashFlag<br>
        "swap headers, but swapping headers swaps remembered bits and hashes;<br>
         remembered bits must be unswapped and hashes may be unswapped if<br>
         copyHash is false."<br>
        "This variant doesn't tickle a compiler bug in gcc and clang.  See naiveSwapHeaders:and:copyHashF<wbr>lag:"<br>
        <inline: true><br>
+       | header1 header2 remembered |<br>
-       | header1 header2 remembered1 remembered2 |<br>
        header1 := self long64At: obj1.<br>
        header2 := self long64At: obj2.<br>
+       remembered := (header1 bitXor: header2) bitAnd: 1 << self rememberedBitShift.<br>
+       remembered ~= 0 ifTrue:<br>
+               [header1 := header1 bitXor: remembered.<br>
+                header2 := header2 bitXor: remembered].<br></blockquote><div><br></div><div>+1 for this one, much more clever as the selector tells AND much more robust</div><div>(previous signed arithmetic could overflow, which might work but is UB).</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
-       remembered1 := header1 bitAnd: 1 << self rememberedBitShift.<br>
-       remembered2 := header2 bitAnd: 1 << self rememberedBitShift.<br>
-       remembered1 ~= remembered2 ifTrue:<br>
-               [header1 := header1 - remembered1 + remembered2.<br>
-                header2 := header2 - remembered2 + remembered1].<br>
        "swapping headers swaps hash; if not copyHashFlag then unswap hash"<br>
        copyHashFlag ifFalse:<br>
+               [| hashBits |<br>
+                hashBits := (header1 bitXor: header2) bitAnd: self identityHashFullWordMask.<br>
+                hashBits ~= 0 ifTrue:<br>
+                       [header1 := header1 bitXor: hashBits.<br>
+                        header2 := header2 bitXor: hashBits]].<br>
-               [| hash1 hash2 |<br>
-                hash1 := header1 bitAnd: self identityHashFullWordMask.<br>
-                hash2 := header2 bitAnd: self identityHashFullWordMask.<br>
-                hash1 ~= hash2 ifTrue:<br>
-                       [header1 := header1 - hash1 + hash2.<br>
-                        header2 := header2 - hash2 + hash1]].<br>
        self long64At: obj1 put: header2.<br>
        self long64At: obj2 put: header1!<br>
<br>
Item was changed:<br>
  ----- Method: SpurMemoryManager>>currentAllo<wbr>catedBytes (in category 'allocation accounting') -----<br>
  currentAllocatedBytes<br>
        "Compute the current allocated bytes since last set.<br>
         This is the cumulative total in statAllocatedBytes plus the allocation since the last scavenge."<br>
        | use |<br>
+       "Slang infers the type of the difference between two unsigned variables as signed.<br>
+        In this case we want it to be unsigned."<br>
+       <var: 'use' type: #usqInt><br>
        use := segmentManager totalOldSpaceCapacity - totalFreeOldSpace.<br>
        ^statAllocatedBytes<br>
         + (freeStart - scavenger eden start)<br>
         + (use - oldSpaceUsePriorToScavenge)!<br>
<br>
</blockquote></div><br></div></div>
</div></blockquote></div></body></html>