llvm dev - Feb 2019 - AVR is little endian, but requires function arguments to be in a "big endian" order, might need an additional data layout variable unless someone can suggest a better fix?

I think this is broken in at least one place when legalising the DAG.
This llvm ir:
%3 = call { i16, i1 } @llvm.umul.with.overflow.i16(i16 %2, i16 11)
Fails to lower correctly on AVR but the problem is, unfortunately, not just
coming from the AVR Target code and I am not sure it can be cleanly fixed just
there. (But I would be very happy to be proved wrong as I'm very new to
this.)

The above code is taken by the legalizer and turned into a call to __mulsi3, but
turns the 16 bit parameters into 32 bit parameters by adding 16 bits of 0 to the
top 16 bits of each.
Unfortunately, to do this, it changes the code to be a call to __mulsi3 with
four 16 bit parameters instead of two 32 bit parameters. It orders these
parameters based on the endianness of the platform, read from the data layout.
Unfortunately that doesn't match the AVR ABI, which expects 32 bit
parameters to be passed in four consecutive registers, yet generally takes
parameters in a sort of "big endian" order.
Details can be found on my bug report to the AVR team:
https://github.com/avr-rust/rust/issues/129
In order to get around this, the only way to deal with this I can think of is to
add an extra field to the data layout for ABIEndianness or something like that,
because otherwise the information that these were 32 bit parameters, not two 16
bit parameters has been lost after legalization.

Any help much appreciated!
 
Thanks,
Carl
 
p.s. I came up with a sample/demo PR for the sort of thing I'm talking
about. Definitely not a finished product, just for discussion:
https://github.com/avr-rust/llvm/pull/8.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190219/0e644992/attachment.html>

Adding a method to TargetLowering makes sense, sure.  (I don’t think we need to
modify the IR because the difference doesn’t affect IR optimizations.)

-Eli

From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of
carl-llvm-dev at petosoft.com via llvm-dev
Sent: Tuesday, February 19, 2019 12:38 PM
To: LLVM Developers Mailing List <llvm-dev at lists.llvm.org>
Subject: [EXT] [llvm-dev] AVR is little endian, but requires function arguments
to be in a "big endian" order, might need an additional data layout
variable unless someone can suggest a better fix?


I think this is broken in at least one place when legalising the DAG.

This llvm ir:
%3 = call { i16, i1 } @llvm.umul.with.overflow.i16(i16 %2, i16 11)

Fails to lower correctly on AVR but the problem is, unfortunately, not just
coming from the AVR Target code and I am not sure it can be cleanly fixed just
there. (But I would be very happy to be proved wrong as I'm very new to
this.)

The above code is taken by the legalizer and turned into a call to __mulsi3, but
turns the 16 bit parameters into 32 bit parameters by adding 16 bits of 0 to the
top 16 bits of each.

Unfortunately, to do this, it changes the code to be a call to __mulsi3 with
four 16 bit parameters instead of two 32 bit parameters. It orders these
parameters based on the endianness of the platform, read from the data layout.
Unfortunately that doesn't match the AVR ABI, which expects 32 bit
parameters to be passed in four consecutive registers, yet generally takes
parameters in a sort of "big endian" order.

Details can be found on my bug report to the AVR team:
https://github.com/avr-rust/rust/issues/129

In order to get around this, the only way to deal with this I can think of is to
add an extra field to the data layout for ABIEndianness or something like that,
because otherwise the information that these were 32 bit parameters, not two 16
bit parameters has been lost after legalization.

Any help much appreciated!



Thanks,

Carl



p.s. I came up with a sample/demo PR for the sort of thing I'm talking
about. Definitely not a finished product, just for discussion:
https://github.com/avr-rust/llvm/pull/8.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190219/15262240/attachment.html>

Cool, looking into that as an option, sounds ideal. Thanks for the steer. :)
 
I'll get back to you guys with results and a patch (probably via the
maintainer, Dylan).
 
Carl
 
-----Original Message-----
From: "Eli Friedman" <efriedma at quicinc.com>
Sent: Tuesday, February 19, 2019 3:44pm
To: "carl-llvm-dev at petosoft.com" <carl-llvm-dev at
petosoft.com>, "LLVM Development List" <llvm-dev at
lists.llvm.org>
Subject: RE: [llvm-dev] AVR is little endian, but requires function arguments to
be in a "big endian" order, might need an additional data layout
variable unless someone can suggest a better fix?




Adding a method to TargetLowering makes sense, sure.  (I don’t think we need to
modify the IR because the difference doesn’t affect IR optimizations.)
 
-Eli
 



From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of
carl-llvm-dev at petosoft.com via llvm-dev
Sent: Tuesday, February 19, 2019 12:38 PM
To: LLVM Developers Mailing List <llvm-dev at lists.llvm.org>
Subject: [EXT] [llvm-dev] AVR is little endian, but requires function arguments
to be in a "big endian" order, might need an additional data layout
variable unless someone can suggest a better fix?
 
I think this is broken in at least one place when legalising the DAG.
This llvm ir:
 %3 = call { i16, i1 } @llvm.umul.with.overflow.i16(i16 %2, i16 11)
Fails to lower correctly on AVR but the problem is, unfortunately, not just
coming from the AVR Target code and I am not sure it can be cleanly fixed just
there. (But I would be very happy to be proved wrong as I'm very new to
this.)

 The above code is taken by the legalizer and turned into a call to __mulsi3,
but turns the 16 bit parameters into 32 bit parameters by adding 16 bits of 0 to
the top 16 bits of each.
Unfortunately, to do this, it changes the code to be a call to __mulsi3 with
four 16 bit parameters instead of two 32 bit parameters. It orders these
parameters based on the endianness of the platform, read from the data layout.
Unfortunately that doesn't match the AVR ABI, which expects 32 bit
parameters to be passed in four consecutive registers, yet generally takes
parameters in a sort of "big endian" order.
Details can be found on my bug report to the AVR team: [
https://github.com/avr-rust/rust/issues/129 ](
https://github.com/avr-rust/rust/issues/129 )
In order to get around this, the only way to deal with this I can think of is to
add an extra field to the data layout for ABIEndianness or something like that,
because otherwise the information that these were 32 bit parameters, not two 16
bit parameters has been lost after legalization.

 Any help much appreciated!
 
Thanks,
Carl
 
p.s. I came up with a sample/demo PR for the sort of thing I'm talking
about. Definitely not a finished product, just for discussion: [
https://github.com/avr-rust/llvm/pull/8 ](
https://github.com/avr-rust/llvm/pull/8 ).
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190220/322fab02/attachment-0001.html>