[Vm-dev] Plugin code generation issues

Eliot Miranda eliot.miranda at gmail.com
Tue Mar 10 06:49:20 UTC 2020


Hi Pierre,

> 
> On Mar 9, 2020, at 11:30 PM, hogoww <pierre_misse25 at msn.com> wrote:
> 
> 
> Hi,
> 
> I did not read this thread in detail, but might it be related to the tests that i shared ?
> (by mail sadly, was not able to find the time to commit them).
> 

I have them; thank you.  As soon as time allows I’ll try and integrate them. They are much appreciated.
> Pierre
> 
> On 09/03/2020 22:21, Nicolas Cellier wrote:
>>  
>> 
>> 
>> Hi Levente,
>> IMO the undue sqInt cast indeed qualifies as a BUG, we were just lucky enough to not encounter it.
>> (said another way, since we have not much tests, we would have corrected it only if encountered it).
>> 
>> This happens in #generateShiftRight:on:indent: because the type analysis is insufficient.
>> You can see that we care to verify if operand is already 64 bits with
>>     (self is64BitIntegralVariable: msgNode receiver typeInto: [:t| type := t])
>> Unfortunately, as the name tells, this method restrict the analysis to the case where
>>     node isVariable
>> 
>> Note how the analysis of operand type is more careful in generateShiftLeft:on:indent:
>> We changed it several times in 2016 with Eliot, and I finally brought current complexification circa July 2016
>> See for example http://source.squeak.org/VMMaker/VMMaker.oscog-nice.1904.diff
>> (some details improving handling of literal constants have changed since).
>> 
>> It uses more general C expression type #typeFor:in: rather than just C variable type.
>> Eliot wrote this more generic type analyzer and I completed it in 2016, this is posterior the #generateShiftRight:on:indent: time stamp, hence the limitations of #generateShiftRight:on:indent. As said above, we did not require a better one until now...
>> 
>> So it seems like we have enough material to correct the generation and make it right.
>> We should really pick the Unit Test initiative from Pharo Team.
>> A pity, a totally unfair attitude, and a total waste that it's not retro contributed... 
>> 
>> 
>> 
>> Le lun. 9 mars 2020 à 18:27, Levente Uzonyi <leves at caesar.elte.hu> a écrit :
>>>  Hi Eliot,
>>> 
>>> On Mon, 9 Mar 2020, Eliot Miranda wrote:
>>> 
>>> > 
>>> > Hi Levente,
>>> >
>>> >
>>> >> On Mar 9, 2020, at 8:54 AM, Levente Uzonyi <leves at caesar.elte.hu> wrote:
>>> >> 
>>> >> Hi All,
>>> >> 
>>> >> I wrote a new plugin the other day and found two issues with code generation.
>>> >> The first one, which is less important is related to the inline pragma.
>>> >> I expected that methods marked with <inline: true> will either not be generated at all when the code generator inlines all occurrences of them or they'll be generated as inline C functions. But they are generated as regular static functions even though they are never used.
>>> >
>>> > Slang is not always able to inline.  For a example, inlining into conditionals is problematic. For this reason Slang treats <inline: true> as a recommendation.  If you want to insist, eliminate the static functions, and deal with any failures to inline by suitable rewrites then use <inline: #always>.
>>> >
>>> > Slang also obeys <inline: #never> which is useful in other ways (for example reducing code duplication).
>>> 
>>> Thanks. <inline: #always> does just what I need.
>>> 
>>> >
>>> >> The other one, which is more significant is related to type casts.
>>> >> I've got the following method:
>>> >> 
>>> >> rotateRight64: value by: amount
>>> >>
>>> >>    <inline: true>
>>> >>    <var: #value type: #'unsigned long long'>
>>> >>    <returnTypeC: #'unsigned long long'>
>>> >>
>>> >>    ^(value >> amount) bitOr: (value << (64 - amount))
>>> >> 
>>> >> The code using the above method has the variable declaration
>>> >>
>>> >>    <var: #w declareC: 'unsigned long long w[80]'>
>>> >> 
>>> >> The method is used with individual elements of w:
>>> >>
>>> >>    self rotateRight64: (w at: i - 2) by: 61
>>> >> 
>>> >> The generated function gets inlined as expected, but the elements of the array are casted to usqInt before right shifts:
>>> >>
>>> >>    ((((usqInt) (w[i2 - 2])) >> 61) | ((w[i2 - 2]) << (3)))
>>> >> 
>>> >> This is a problem on 32-bit platforms, because usqInt is unsigned int there, so the upper 32-bits will be lost by the cast.
>>> >> 
>>> >> When the method is used with non-array variables, then the generator works as expected:
>>> >>
>>> >>    self rotateRight64: a by: 39
>>> >> 
>>> >> is generated as:
>>> >>
>>> >>    ((a >> 39) | (a << (25)))
>>> >> 
>>> >> where a is declared as
>>> >>
>>> >>    <var: #a type: #'unsigned long long'>
>>> >> 
>>> >> I tried to fix the issue by declaring w as
>>> >>
>>> >>    <var: #w type: #'unsigned long long*'>
>>> >> 
>>> >> But is didn't make the usqInt casts go away.
>>> >> Is it possible that the code generator doesn't know that the type of w[i] is unsigned long long, when w is unsigned long long*?
>>> >
>>> > I’ll need to take a look at this in detail.  For now, try using the type #usqLong and see if it makes a difference.
>>> 
>>> Unfortunately, the code behaves the same way with #usqLong.
>>> 
>>> I forgot to mention a third issue. There's no easy way to check the type 
>>> of 64-bit and 16-bit arrays. There's #isWords: for 32-bit arrays, and 
>>> #isBytes: for 8-bit arrays.
>>> #isWordsOrBytes: is a misnomer now, because it returns true for 8, 16, 32 
>>> and 64-bit arrays.
>>> 
>>> I used the following workaround:
>>> 
>>>         ((interpreterProxy isIndexable: hashOop)
>>>                 and: [ (interpreterProxy isWordsOrBytes: hashOop)
>>>                 and: [ (interpreterProxy slotSizeOf: hashOop) = 8 ] ])
>>>                 ifFalse: [ ^interpreterProxy primitiveFailFor: PrimErrBadArgument ].
>>> 
>>> The first check is probably unnecessary.
>>> 
>>> SpurMemoryManager understands #isLong64s:, but InterpreterProxy doesn't 
>>> have that method.
>>> Also, some implementations in InterpreterProxy may be incorrect. e.g. 
>>> #isWordsOrBytes:.
>>> 
>>> 
>>> Levenete
>>> 
>>> >
>>> >> Levente
>>> >> 
>>> >> P.S.: The code is available here http://squeaksource.com/Cryptography/CryptographyPlugins-ul.18.mcz .
>>> >> Generate SHA2Plugin to see the problem.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20200309/5e98fb9d/attachment.html>


More information about the Vm-dev mailing list