[Vm-dev] Funny Floats [WAS OpenSmalltalk current on aarch64]

[Eliot Miranda]

Hi Nicolas,

On Wed, Dec 11, 2019 at 11:58 AM Nicolas Cellier <
nicolas.cellier.aka.nice at gmail.com> wrote:

>
> Hi all, Eliot, Pablo,
> we should review all the pointer aliasing that we are still depending
> upon, because it can strike anytime soon...
> The recommended way is memcpy, so let's use that.
> The alternative is type puning via union, but it's not legal on C++. If
> wanting to support exotic compiler (MSVC) which is mainly focused on C++,
> not modern C, it's safer to use memcpy.
>

Agreed.  I just went through all the uses of casts to int * and verified
that all the other uses are legitimate (they're typically cases of
firstIndexableField:, so of something in memory, not something that may be
in a register).

That's what this issue is really about.  In the olden days for a C compiler
to put something in a register one had to use the register type qualifier,
and taking the address of an automatic variable (C's name for a local
variable) would raise an error.  Then (quite rightly) gcc started ignoring
the register qualifier and (at my request) added the asm("register name")
form for specifying global register variables. Taking the address of an
automatic variable then marked it as a variable that couldn't be assigned
to a variable.  So tat was fine.  One could perfectly legally play the
usual memory punning tricks without having to resort to writing a union.

When 64-bit computing went mainstream the need was for C compilers to
generate good code for int variables and so lots of previously valid code
was redefined to be undefined behavior.  See Chris Lattner's excellent blog
post http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html.
 (While this is a really good post I don't endorse the position taken by C
compilers; IMO this is an example of the tail wagging the dog; compilers
could still do what they did in the '90's and stack allocate something
whose address is taken).  Anyway, we don't control this world so we have to
adapt.


>
> Le mer. 11 déc. 2019 à 20:00, Eliot Miranda <eliot.miranda at gmail.com> a
> écrit :
>
>>
>> > On Dec 11, 2019, at 9:09 AM, "tesonep at gmail.com" <tesonep at gmail.com>
>> wrote:
>> > 
>> > Hi Eliot, you can use PT as initials (I don't if there is a clash or
>> > ptesone) and the timestamp is just today.
>>
>> Thanks.  And I’ll include your email on the GCC SSA.  Thanks for tracking
>> this down.  This has been biting us for some time.  There was a similar
>> issue with accessing the two 32-bit halves of the 64-bit object header in
>> Spur that meant I had to use a rewrite.  I’ll try and dig it out.  Perhaps
>> we could collaborate on performing the same analysis and verifying that
>> there is an issue with this header access code in gcc.
>>
>> >
>> > Thanks
>> >
>> >> On Wed, Dec 11, 2019 at 6:02 PM Eliot Miranda <eliot.miranda at gmail.com>
>> wrote:
>> >>
>> >>
>> >>
>> >>
>> >>>> On Dec 11, 2019, at 8:45 AM, "tesonep at gmail.com" <tesonep at gmail.com>
>> wrote:
>> >>>
>> >>> 
>> >>> Hi Ken,
>> >>> this is a problem in an optimization of GCC 8.3 (I am not sure what
>> >>> other versions are impacted).
>> >>> Basically it is generating bad a function, removing code that assumes
>> >>> that is dead code.
>> >>> Even though this is a bug in GCC, because there is not actual reason
>> >>> to remove the code.
>> >>> It is produced because the code is not written in a nice way.
>> >>>
>> >>> Basically, if someone wants to fix it in the VM code is modifying a
>> >>> single method:
>> >>>
>> >>> Spur64BitMemoryManager >> fetchLong32: fieldIndex ofFloatObject: oop
>> >>>   "index by word size, and return a pointer as long as the word size"
>> >>>
>> >>>   | bits |
>> >>>   (self isImmediateFloat: oop) ifFalse:
>> >>>       [^self fetchLong32: fieldIndex ofObject: oop].
>> >>>   bits := self smallFloatBitsOf: oop.
>> >>>   ^ fieldIndex = 0
>> >>>       ifTrue: [bits bitAnd: 16rFFFFFFFF]
>> >>>       ifFalse: [bits >> 32]
>> >>
>> >> Thank you, Pablo.  I’ll integrate this quickly.  Can you possibly give
>> me initials and time stamp?  It’s not necessary but helps that the code is
>> marked as being your fix.
>> >>
>> >>>
>> >>> The old method had a handwritten piece of C code to replace the last
>> if:
>> >>>
>> >>> (self cCoerceSimple: (self addressOf: bits) to: #'int *') at:
>> fieldIndex
>> >>>
>> >>> I assume it was done to handle a different kind of endianness, but the
>> >>> single sender of this message is already doing the handling and having
>> >>> different behavior depending on the endianness.
>> >>>
>> >>> With this fix the build can still be executed with -O2 optimization
>> in GCC 8.3
>> >>>
>> >>> Cheers,
>> >>>
>> >>>> On Sat, Dec 7, 2019 at 4:20 PM <ken.dickey at whidbey.com> wrote:
>> >>>>
>> >>>>
>> >>>>> Have you tried switching off compiler optimisations?
>> >>>>
>> >>>> Ah!  I had forgotten that one.  I thought that was changed in mvm.
>> >>>>
>> >>>> Image works fine noe (but more slowly!) on aarch64 Alpine Linux
>> >>>> (musl+busybox).
>> >>>>
>> >>>> I am just out the door fir a couple of days but will try fdlib on
>> >>>> return.
>> >>>>
>> >>>> Thanks again to all!
>> >>>> -KenD
>> >>>
>> >>>
>> >>>
>> >>> --
>> >>> Pablo Tesone.
>> >>> tesonep at gmail.com
>> >
>> >
>> >
>> > --
>> > Pablo Tesone.
>> > tesonep at gmail.com
>>
>

-- 
_,,,^..^,,,_
best, Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20191211/b604de53/attachment.html>