<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<p>Hi,</p>
<p>I did not read this thread in detail, but might it be related to
the tests that i shared ?<br>
(by mail sadly, was not able to find the time to commit them).</p>
<p>Pierre<br>
</p>
<div class="moz-cite-prefix">On 09/03/2020 22:21, Nicolas Cellier
wrote:<br>
</div>
<blockquote type="cite" cite="mid:CAKnRiT4EqqJEuLr4bg1nghPCigC4E+CGCur4AowDYHcifznzeg@mail.gmail.com">
<pre class="moz-quote-pre" wrap=""> </pre>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<div dir="ltr">
<div>Hi Levente,
<div>IMO the undue sqInt cast indeed qualifies as a BUG, we
were just lucky enough to not encounter it.</div>
<div>(said another way, since we have not much tests, we would
have corrected it only if encountered it).</div>
<div><br>
</div>
<div>This happens in #generateShiftRight:on:indent: because
the type analysis is insufficient.</div>
<div>You can see that we care to verify if operand is already
64 bits with</div>
<div> (self is64BitIntegralVariable: msgNode receiver
typeInto: [:t| type := t])</div>
<div>Unfortunately, as the name tells, this method restrict
the analysis to the case where</div>
<div> node isVariable</div>
<div><br>
</div>
</div>
<div>Note how the analysis of operand type is more careful in
generateShiftLeft:on:indent:
<div>We changed it several times in 2016 with Eliot, and I
finally brought current complexification circa July 2016</div>
<div>See for example <a href="http://source.squeak.org/VMMaker/VMMaker.oscog-nice.1904.diff" moz-do-not-send="true">http://source.squeak.org/VMMaker/VMMaker.oscog-nice.1904.diff</a></div>
<div>(some details improving handling of literal constants
have changed since).</div>
<div><br>
</div>
<div>It uses more general C expression type #typeFor:in:
rather than just C variable type.</div>
<div>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...<br>
</div>
</div>
<div><br>
</div>
<div>So it seems like we have enough material to correct the
generation and make it right.</div>
<div>We should really pick the Unit Test initiative from Pharo
Team.</div>
<div>A pity, a totally unfair attitude, and a total waste that
it's not retro contributed... <br>
</div>
<div><br>
</div>
<br>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">Le lun. 9 mars 2020 à 18:27,
Levente Uzonyi <<a href="mailto:leves@caesar.elte.hu" moz-do-not-send="true">leves@caesar.elte.hu</a>> a
écrit :<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Hi
Eliot,<br>
<br>
On Mon, 9 Mar 2020, Eliot Miranda wrote:<br>
<br>
> <br>
> Hi Levente,<br>
><br>
><br>
>> On Mar 9, 2020, at 8:54 AM, Levente Uzonyi <<a href="mailto:leves@caesar.elte.hu" target="_blank" moz-do-not-send="true">leves@caesar.elte.hu</a>> wrote:<br>
>> <br>
>> Hi All,<br>
>> <br>
>> I wrote a new plugin the other day and found two
issues with code generation.<br>
>> The first one, which is less important is related to
the inline pragma.<br>
>> 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.<br>
><br>
> 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>.<br>
><br>
> Slang also obeys <inline: #never> which is useful
in other ways (for example reducing code duplication).<br>
<br>
Thanks. <inline: #always> does just what I need.<br>
<br>
><br>
>> The other one, which is more significant is related
to type casts.<br>
>> I've got the following method:<br>
>> <br>
>> rotateRight64: value by: amount<br>
>><br>
>> <inline: true><br>
>> <var: #value type: #'unsigned long long'><br>
>> <returnTypeC: #'unsigned long long'><br>
>><br>
>> ^(value >> amount) bitOr: (value <<
(64 - amount))<br>
>> <br>
>> The code using the above method has the variable
declaration<br>
>><br>
>> <var: #w declareC: 'unsigned long long
w[80]'><br>
>> <br>
>> The method is used with individual elements of w:<br>
>><br>
>> self rotateRight64: (w at: i - 2) by: 61<br>
>> <br>
>> The generated function gets inlined as expected, but
the elements of the array are casted to usqInt before right
shifts:<br>
>><br>
>> ((((usqInt) (w[i2 - 2])) >> 61) | ((w[i2 -
2]) << (3)))<br>
>> <br>
>> 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.<br>
>> <br>
>> When the method is used with non-array variables,
then the generator works as expected:<br>
>><br>
>> self rotateRight64: a by: 39<br>
>> <br>
>> is generated as:<br>
>><br>
>> ((a >> 39) | (a << (25)))<br>
>> <br>
>> where a is declared as<br>
>><br>
>> <var: #a type: #'unsigned long long'><br>
>> <br>
>> I tried to fix the issue by declaring w as<br>
>><br>
>> <var: #w type: #'unsigned long long*'><br>
>> <br>
>> But is didn't make the usqInt casts go away.<br>
>> 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*?<br>
><br>
> 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.<br>
<br>
Unfortunately, the code behaves the same way with #usqLong.<br>
<br>
I forgot to mention a third issue. There's no easy way to
check the type <br>
of 64-bit and 16-bit arrays. There's #isWords: for 32-bit
arrays, and <br>
#isBytes: for 8-bit arrays.<br>
#isWordsOrBytes: is a misnomer now, because it returns true
for 8, 16, 32 <br>
and 64-bit arrays.<br>
<br>
I used the following workaround:<br>
<br>
((interpreterProxy isIndexable: hashOop)<br>
and: [ (interpreterProxy isWordsOrBytes:
hashOop)<br>
and: [ (interpreterProxy slotSizeOf: hashOop)
= 8 ] ])<br>
ifFalse: [ ^interpreterProxy primitiveFailFor:
PrimErrBadArgument ].<br>
<br>
The first check is probably unnecessary.<br>
<br>
SpurMemoryManager understands #isLong64s:, but
InterpreterProxy doesn't <br>
have that method.<br>
Also, some implementations in InterpreterProxy may be
incorrect. e.g. <br>
#isWordsOrBytes:.<br>
<br>
<br>
Levenete<br>
<br>
><br>
>> Levente<br>
>> <br>
>> P.S.: The code is available here <a href="http://squeaksource.com/Cryptography/CryptographyPlugins-ul.18.mcz" rel="noreferrer" target="_blank" moz-do-not-send="true">http://squeaksource.com/Cryptography/CryptographyPlugins-ul.18.mcz</a>
.<br>
>> Generate SHA2Plugin to see the problem.</blockquote>
</div>
</blockquote>
</body>
</html>