Skip to content

Commit 3c474f3

Browse files
dylanmckaydylanmckay
committed
[AVR] Reverse split return values to into the "big endian" format required by the ABI
When returning structs, the fields need to be reversed. Without this, the ABI of the generated executables is not compatible with avr-gcc, nor will LLVM allow it to be compiled. Here is the assertion message when complex structs are used: Impossible reg-to-reg copy UNREACHABLE executed at llvm/lib/Target/AVR/AVRInstrInfo.cpp! This was first noticed in avr-rust/rust-legacy-fork#92. Description by Peter The AVR couldn't handle returning structs like this at all (see avr-rust/rust-legacy-fork#57). It only expects to ever return an iXX and only when XX is >=32 it needs to reverse the registers because of the calling convention. Now for this bug where we have { i8, i32 } and registers {[0] = i8, [1] = i16, [2] = i16} it only should reverse registers 1 and 2 but not 0. Patch by Peter Nimmervoll (@brainlag) and Tim Neumann (@TimNN)
1 parent 2f70705 commit 3c474f3

File tree

7 files changed

+205
-25
lines changed

7 files changed

+205
-25
lines changed

‎docs/AVRTarget.rst

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
=================================
2+
Documentation for the AVR backend
3+
=================================
4+
5+
.. contents::
6+
:local:
7+
8+
Calling convention
9+
==================
10+
11+
External links
12+
--------------
13+
14+
* The avr-gcc .. _wiki: https://gcc.gnu.org/wiki/avr-gcc#Calling_Convention has a description of the C calling convention they use
15+
16+
* The avr-libc .. documentation: http://www.nongnu.org/avr-libc/user-manual/FAQ.html#faq_reg_usage has a small summary of the C calling convention
17+
18+
19+
Function return values
20+
----------------------
21+
22+
The avr-gcc documentation states that
23+
24+
Return values with a size of 1 byte up to and including a size of 8 bytes will be returned in registers. Return values whose size is outside that range will be returned in memory.
25+
26+
-- avr-gcc wiki, October 2018
27+
28+
This does not strictly seem to be the case. Scalar return values like i8, i16, and i64 do indeed seem to follow this rule, according to the output of avr-gcc. Integer return values of up to 64-bits (8 bytes) are always returned directly in registers. However, when a struct is being returned, only structs of 4 bytes or less are returned in registers. If the struct is of 5 bytes or larger, avr-gcc will always pass it on the stack.
29+
30+
This means that different return types have different size limits for fitting into registers or the stack.

‎docs/index.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,7 @@ For API clients and LLVM developers.
285285
HowToUseAttributes
286286
NVPTXUsage
287287
AMDGPUUsage
288+
AVRTarget
288289
StackMaps
289290
InAlloca
290291
BigEndianNEON
@@ -390,6 +391,10 @@ For API clients and LLVM developers.
390391
:doc:`AMDGPUUsage`
391392
This document describes using the AMDGPU backend to compile GPU kernels.
392393

394+
:doc:`AVRTarget`
395+
This document contains pieces of information an AVR compiler developer may
396+
want to know.
397+
393398
:doc:`StackMaps`
394399
LLVM support for mapping instruction addresses to the location of
395400
values and allowing code to be patched.

‎lib/Target/AVR/AVRISelLowering.cpp

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1298,6 +1298,35 @@ SDValue AVRTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
12981298
InVals);
12991299
}
13001300

1301+
/// Reverse splitted return values to get the "big endian" format required
1302+
/// to agree with the calling convention ABI.
1303+
static void ReverseArgumentsToBigEndian(MachineFunction &MF,
1304+
SmallVector<CCValAssign, 16> &RVLocs) {
1305+
if (RVLocs.size() > 1) {
1306+
// Some hackery because SelectionDAGBuilder does not split
1307+
// up arguments properly
1308+
Type *retType = MF.getFunction().getReturnType();
1309+
if (retType->isStructTy()) {
1310+
if (retType->getNumContainedTypes() > 1 &&
1311+
retType->getNumContainedTypes() > RVLocs.size()) {
1312+
for (unsigned i = 0, pos = 0;
1313+
i < retType->getNumContainedTypes(); ++i) {
1314+
Type *field = retType->getContainedType(i);
1315+
if(field->isIntegerTy() && field->getIntegerBitWidth() > 16) {
1316+
int Size = field->getIntegerBitWidth() / 16;
1317+
std::reverse(RVLocs.begin()+ pos, RVLocs.end() + pos + Size);
1318+
pos += Size;
1319+
} else {
1320+
pos++;
1321+
}
1322+
}
1323+
}
1324+
} else {
1325+
std::reverse(RVLocs.begin(), RVLocs.end());
1326+
}
1327+
}
1328+
}
1329+
13011330
/// Lower the result values of a call into the
13021331
/// appropriate copies out of appropriate physical registers.
13031332
///
@@ -1315,12 +1344,6 @@ SDValue AVRTargetLowering::LowerCallResult(
13151344
auto CCFunction = CCAssignFnForReturn(CallConv);
13161345
CCInfo.AnalyzeCallResult(Ins, CCFunction);
13171346

1318-
if (CallConv != CallingConv::AVR_BUILTIN && RVLocs.size() > 1) {
1319-
// Reverse splitted return values to get the "big endian" format required
1320-
// to agree with the calling convention ABI.
1321-
std::reverse(RVLocs.begin(), RVLocs.end());
1322-
}
1323-
13241347
// Copy all of the result registers out of their specified physreg.
13251348
for (CCValAssign const &RVLoc : RVLocs) {
13261349
Chain = DAG.getCopyFromReg(Chain, dl, RVLoc.getLocReg(), RVLoc.getValVT(),
@@ -1383,9 +1406,7 @@ AVRTargetLowering::LowerReturn(SDValue Chain, CallingConv::ID CallConv,
13831406

13841407
// Reverse splitted return values to get the "big endian" format required
13851408
// to agree with the calling convention ABI.
1386-
if (e > 1) {
1387-
std::reverse(RVLocs.begin(), RVLocs.end());
1388-
}
1409+
ReverseArgumentsToBigEndian(MF, RVLocs);
13891410

13901411
SDValue Flag;
13911412
SmallVector<SDValue, 4> RetOps(1, Chain);

‎test/CodeGen/AVR/call.ll

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -172,15 +172,17 @@ define void @testcallprologue() {
172172
define i32 @icall(i32 (i32)* %foo) {
173173
; CHECK-LABEL: icall:
174174
; CHECK: movw r30, r24
175-
; CHECK: ldi r22, 147
176-
; CHECK: ldi r23, 248
177-
; CHECK: ldi r24, 214
178-
; CHECK: ldi r25, 198
179-
; CHECK: icall
180-
; CHECK: subi r22, 251
181-
; CHECK: sbci r23, 255
182-
; CHECK: sbci r24, 255
183-
; CHECK: sbci r25, 255
175+
; CHECK-NEXT: ldi r22, 147
176+
; CHECK-NEXT: ldi r23, 248
177+
; CHECK-NEXT: ldi r24, 214
178+
; CHECK-NEXT: ldi r25, 198
179+
; CHECK-NEXT: icall
180+
; CHECK-NEXT: movw r18, r22
181+
; CHECK-NEXT: subi r24, 251
182+
; CHECK-NEXT: sbci r25, 255
183+
; CHECK-NEXT: sbci r18, 255
184+
; CHECK-NEXT: sbci r19, 255
185+
184186
%1 = call i32 %foo(i32 3335977107)
185187
%2 = add nsw i32 %1, 5
186188
ret i32 %2
@@ -200,10 +202,12 @@ define i32 @externcall(float %a, float %b) {
200202
; CHECK: movw r20, [[REG1]]
201203
; CHECK: call __divsf3
202204
; CHECK: call foofloat
203-
; CHECK: subi r22, 251
204-
; CHECK: sbci r23, 255
205-
; CHECK: sbci r24, 255
205+
; CHECK: movw r18, r22
206+
; CHECK: subi r24, 251
206207
; CHECK: sbci r25, 255
208+
; CHECK: sbci r18, 255
209+
; CHECK: sbci r19, 255
210+
207211
%1 = fdiv float %b, %a
208212
%2 = call i32 @foofloat(float %1)
209213
%3 = add nsw i32 %2, 5

‎test/CodeGen/AVR/calling-conv/c/return.ll

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,117 @@ define i64 @ret_i64() {
3434
; CHECK-NEXT: mov r25, r19
3535
ret i64 18374966859414961920
3636
}
37+
38+
; CHECK-LABEL: ret_struct1
39+
define { i8, i32 } @ret_struct1() {
40+
start:
41+
; CHECK: ldi r22, 221
42+
; CHECK: ldi r23, 204
43+
; CHECK: ldi r20, 187
44+
; CHECK: ldi r21, 170
45+
; CHECK: ldi r24, 111
46+
ret { i8, i32 } { i8 111, i32 2864434397 } ; { 0x6f, 0xaabbccdd }
47+
}
48+
49+
; CHECK-LABEL: ret_struct2
50+
define { i8, i8, i8, i8, i8 } @ret_struct2() {
51+
start:
52+
ret { i8, i8, i8, i8, i8 } { i8 111, i8 221, i8 204, i8 187, i8 170 } ; { 0x6f, 0xaabbccdd }
53+
}
54+
55+
; CHECK-LABEL: ret_struct3
56+
define { i32, i32 } @ret_struct3() {
57+
start:
58+
ret { i32, i32 } zeroinitializer
59+
}
60+
61+
; CHECK-LABEL: ret_struct4
62+
define { i8, i64 } @ret_struct4() {
63+
start:
64+
ret { i8, i64 } zeroinitializer
65+
}
66+
67+
; CHECK-LABEL: ret_struct5
68+
define { i8, i32, i16 } @ret_struct5() {
69+
start:
70+
ret { i8, i32, i16 } zeroinitializer
71+
}
72+
73+
; CHECK-LABEL: ret_struct6
74+
define { i8, i32, i32 } @ret_struct6() {
75+
start:
76+
ret { i8, i32, i32 } zeroinitializer
77+
}
78+
79+
; Structs of size 1 should be returned in registers.
80+
;
81+
; This matches avr-gcc behaviour.
82+
;
83+
; CHECK-LABEL: ret_struct_size1
84+
define { i8 } @ret_struct_size1() {
85+
; CHECK: ldi r24, 123
86+
ret { i8 } { i8 123 }
87+
}
88+
89+
; Structs of size 2 are currently returned on the stack.
90+
;
91+
; BUG(PR39251): avr-gcc returns all structs of four bytes or less in registers.
92+
;
93+
; CHECK-LABEL: ret_struct_size2
94+
define { i8, i8 } @ret_struct_size2() {
95+
ret { i8, i8 } { i8 123, i8 234 }
96+
}
97+
98+
; Structs of size 3 are currently returned on the stack.
99+
;
100+
; BUG(PR39251): avr-gcc returns all structs of four bytes or less in registers.
101+
;
102+
; CHECK-LABEL: ret_struct_size3
103+
define { i8, i8, i8 } @ret_struct_size3() {
104+
ret { i8, i8, i8 } { i8 123, i8 234, i8 255 }
105+
}
106+
107+
; Structs of size 4 are currently returned on the stack.
108+
;
109+
; BUG(PR39251): avr-gcc returns all structs of four bytes or less in registers.
110+
;
111+
; CHECK-LABEL: ret_struct_size4
112+
define { i8, i8, i8, i8 } @ret_struct_size4() {
113+
ret { i8, i8, i8, i8 } { i8 123, i8 234, i8 255, i8 11 }
114+
}
115+
116+
; Structs of size 5 should be returned on the stack.
117+
;
118+
; This matches avr-gcc behaviour.
119+
;
120+
; CHECK-LABEL: ret_struct_size5
121+
define { i8, i8, i8, i8, i8 } @ret_struct_size5() {
122+
ret { i8, i8, i8, i8, i8 } { i8 123, i8 234, i8 255, i8 11, i8 22 }
123+
}
124+
125+
; Structs of size 6 should be returned on the stack.
126+
;
127+
; This matches avr-gcc behaviour.
128+
;
129+
; CHECK-LABEL: ret_struct_size6
130+
define { i8, i8, i8, i8, i8, i8 } @ret_struct_size6() {
131+
ret { i8, i8, i8, i8, i8, i8 } { i8 123, i8 234, i8 255, i8 11, i8 22, i8 33 }
132+
}
133+
134+
; Structs of size 7 should be returned on the stack.
135+
;
136+
; This matches avr-gcc behaviour.
137+
;
138+
; CHECK-LABEL: ret_struct_size7
139+
define { i8, i8, i8, i8, i8, i8, i8 } @ret_struct_size7() {
140+
ret { i8, i8, i8, i8, i8, i8, i8 } { i8 123, i8 234, i8 255, i8 11, i8 22, i8 33, i8 44 }
141+
}
142+
143+
; Structs of size 8 should be returned on the stack.
144+
;
145+
; This matches avr-gcc behaviour.
146+
;
147+
; CHECK-LABEL: ret_struct_size8
148+
define { i8, i8, i8, i8, i8, i8, i8, i8 } @ret_struct_size8() {
149+
ret { i8, i8, i8, i8, i8, i8, i8, i8 } { i8 123, i8 234, i8 255, i8 11, i8 22, i8 33, i8 44, i8 55 }
150+
}

‎test/CodeGen/AVR/div.ll

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,9 @@ define i16 @sdiv16(i16 %a, i16 %b) {
4444
define i32 @udiv32(i32 %a, i32 %b) {
4545
; CHECK-LABEL: udiv32:
4646
; CHECK: call __udivmodsi4
47-
; CHECK-NEXT: movw r22, r18
48-
; CHECK-NEXT: movw r24, r20
47+
; CHECK-NEXT: movw r18, r22
48+
; CHECK-NEXT: movw r22, r24
49+
; CHECK-NEXT: movw r24, r18
4950
; CHECK-NEXT: ret
5051
%quot = udiv i32 %a, %b
5152
ret i32 %quot
@@ -55,8 +56,9 @@ define i32 @udiv32(i32 %a, i32 %b) {
5556
define i32 @sdiv32(i32 %a, i32 %b) {
5657
; CHECK-LABEL: sdiv32:
5758
; CHECK: call __divmodsi4
58-
; CHECK-NEXT: movw r22, r18
59-
; CHECK-NEXT: movw r24, r20
59+
; CHECK-NEXT: movw r18, r22
60+
; CHECK-NEXT: movw r22, r24
61+
; CHECK-NEXT: movw r24, r18
6062
; CHECK-NEXT: ret
6163
%quot = sdiv i32 %a, %b
6264
ret i32 %quot

‎test/CodeGen/AVR/rem.ll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ define i16 @srem16(i16 %a, i16 %b) {
4242
define i32 @urem32(i32 %a, i32 %b) {
4343
; CHECK-LABEL: urem32:
4444
; CHECK: call __udivmodsi4
45+
; CHECK: movw r22, r20
46+
; CHECK: movw r24, r18
4547
; CHECK-NEXT: ret
4648
%rem = urem i32 %a, %b
4749
ret i32 %rem
@@ -51,6 +53,8 @@ define i32 @urem32(i32 %a, i32 %b) {
5153
define i32 @srem32(i32 %a, i32 %b) {
5254
; CHECK-LABEL: srem32:
5355
; CHECK: call __divmodsi4
56+
; CHECK: movw r22, r20
57+
; CHECK: movw r24, r18
5458
; CHECK-NEXT: ret
5559
%rem = srem i32 %a, %b
5660
ret i32 %rem

0 commit comments

Comments
 (0)