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

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!

Hi,

Yeah, I can see what you’re saying. I think I can probably use it as enough of a hack in this one case by setting an appropriate flag but it’s tricky. I thought I saw IsSplit not being set on a return too but I couldn’t reproduce the problem later.