carl-llvm-dev@petosoft.com via llvm-dev
2019-Feb-19 20:38 UTC
[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/0e644992/attachment.html>
Eli Friedman via llvm-dev
2019-Feb-19 20:44 UTC
[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 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>
carl-llvm-dev@petosoft.com via llvm-dev
2019-Feb-20 10:16 UTC
[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?
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>