[Vm-dev] Plugin code generation issues
hogoww
pierre_misse25 at msn.com
Tue Mar 10 06:30:26 UTC 2020
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).
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
> <mailto: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 <mailto: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/20200310/bcc03e66/attachment-0001.html>
More information about the Vm-dev
mailing list