llvm dev - Mar 2019 - Legalising seems to lose critical information needed for lowering return values properly?

I'm a new LLVM developer contributing patches for the AVR platform and
I'm trying to understand which parts of the code base are malfunctioning in
my case.
 
This LLVM IR...
 
define hidden i32 @setServoAngle3(i32) local_unnamed_addr {
entry:
 %1 = call i32 @myExternalFunction1(i32 %0, i32 119)
 ret i32 %1
}
declare i32 @myExternalFunction1(i32, i32)
 
Is being lowered to this assembly language...
 
setServoAngle3:                         ; @setServoAngle3
; %bb.0:                                ; %entry
 ldi r18, 119
 ldi r19, 0
 ldi r20, 0
 ldi r21, 0
 call myExternalFunction1
 mov r18, r22
 mov r19, r23
 mov r22, r24
 mov r23, r25
 mov r24, r18
 mov r25, r19
 ret
 
Which is clearly wrong.  It should just be...
setServoAngle3:                         ; @setServoAngle3
; %bb.0:                                ; %entry
ldi r18, 119
ldi r19, 0
ldi r20, 0
ldi r21, 0
call myExternalFunction1
ret
 
 
 
 
The AVR ABI is based on the AVR GCC ABI, which specifies for register passing
and returning of arguments, a 32 bit value is passed in r22, r23, r24, r25 (the
registers are 8 bit). And two 16 bit values would be passed, argument 1 in r24,
r25 and argument 2 in r22, r23.
 
 
 
When I'm looking in the function lowering code in
SelectionDAGISel::LowerArguments and
AVRTargetLowering::LowerFormalArguments, we already have a problem because the
32 bit return value has been turned into two 16 bit values by the legaliser and
the information has been lost that it was one 32 bit value. So the lowering code
cannot correctly lower the return value and gets the two 16 bit words mixed up.
 
 
 
Which bit of the code do people think is at fault here? What hooks need changing
on AVR?
 
Thanks for any advice or help you can give.
 
Carl
 
 
 
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190302/184cd2d0/attachment.html>

On Sat, 2 Mar 2019 at 06:58, carl-llvm-dev at petosoft.com via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> AVRTargetLowering::LowerFormalArguments, we already have a problem because
the 32 bit return value has been turned into two 16 bit values by the legaliser
and the information has been lost that it was one 32 bit value.
Isn't it kept (rather awkwardly) by the fact that the first half of
this value has the "IsSplit" flag set? The ARM backends use this to
handle i64 and i128 specially; they need extra alignment rather than
byte-swapping, but the principle ought to be similar.
> Which bit of the code do people think is at fault here? What hooks need
changing on AVR?
That's a bit of a tricky question. The way ABIs work on Clang & LLVM
is unfortunately through a tight coupling between the backend and
Clang CodeGen. As long as Clang is capable of producing compliant code
the question of how natural the interface is becomes QoI.

In this case putting the byte-swapping into Clang would be unnatural
enough that I lean towards it being a backend issue (maybe with
generic involvement depending on how the IsSplit thing works out).

Cheers.

Tim.

Yes, that sounds like the place for me to look.

Is there any documentation or old notes anywhere or do you think I’m best to
just grep the source code for IsSplit and try to copy what ARM does?

Thanks so much Tim!

> On 3 Mar 2019, at 03:08, Tim Northover <t.p.northover at gmail.com>
wrote:
> 
> On Sat, 2 Mar 2019 at 06:58, carl-llvm-dev at petosoft.com via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>> AVRTargetLowering::LowerFormalArguments, we already have a problem
because the 32 bit return value has been turned into two 16 bit values by the
legaliser and the information has been lost that it was one 32 bit value.
> 
> Isn't it kept (rather awkwardly) by the fact that the first half of
> this value has the "IsSplit" flag set? The ARM backends use this
to
> handle i64 and i128 specially; they need extra alignment rather than
> byte-swapping, but the principle ought to be similar.
> 
>> Which bit of the code do people think is at fault here? What hooks need
changing on AVR?
> 
> That's a bit of a tricky question. The way ABIs work on Clang &
LLVM
> is unfortunately through a tight coupling between the backend and
> Clang CodeGen. As long as Clang is capable of producing compliant code
> the question of how natural the interface is becomes QoI.
> 
> In this case putting the byte-swapping into Clang would be unnatural
> enough that I lean towards it being a backend issue (maybe with
> generic involvement depending on how the IsSplit thing works out).
> 
> Cheers.
> 
> Tim.

Hi,

On Sat, 2 Mar 2019 at 06:58, carl-llvm-dev at petosoft.com via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> AVRTargetLowering::LowerFormalArguments, we already have a problem because
the 32 bit return value has been turned into two 16 bit values by the legaliser
and the information has been lost that it was one 32 bit value.
> Isn't it kept (rather awkwardly) by the fact that the first half of
> this value has the "IsSplit" flag set? The ARM backends use this
to
> handle i64 and i128 specially; they need extra alignment rather than
> byte-swapping, but the principle ought to be similar.
Just my two cents; I believe that 'IsSplit' is not actually set on
return values in certain cases. Found this out when I was looking into a similar
calling convention detail in our downstream target.

Also, this might not be relevant to this particular case, but IsSplit is a bit
of a kludge and generally useless if you're splitting something into more
than two parts, since it cannot tell you which sub-value you're actually
looking at. It only tells you that it _is_ a sub-value (and if it's the last
one), which makes picking registers in some particular order quite difficult.
This means you need to keep state in your CC analyzer, or preanalyze the
arguments.

/ Bevin
_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190307/2e4b0953/attachment-0001.html>