Skip to content

Commit c967687

Browse files
Patryk27nikic
authored andcommitted
[AVR] Fix expanding MOVW for overlapping registers
When expanding a MOVW (16-bit copy) to two MOVs (8-bit copy), the lower byte always comes first. This is incorrect for corner cases like '$r24r23 -> $r25r24', in which the higher byte copy should come first. Current patch fixes that bug as recorded at rust-lang/rust#98167 Reviewed By: benshi001 Differential Revision: https://reviews.llvm.org/D128588
1 parent d1ddc34 commit c967687

File tree

3 files changed

+82
-6
lines changed

3 files changed

+82
-6
lines changed

llvm/lib/Target/AVR/AVRInstrInfo.cpp

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,9 @@ void AVRInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
4646
const AVRRegisterInfo &TRI = *STI.getRegisterInfo();
4747
unsigned Opc;
4848

49-
// Not all AVR devices support the 16-bit `MOVW` instruction.
5049
if (AVR::DREGSRegClass.contains(DestReg, SrcReg)) {
50+
// If our AVR has `movw`, let's emit that; otherwise let's emit two separate
51+
// `mov`s.
5152
if (STI.hasMOVW() && AVR::DREGSMOVWRegClass.contains(DestReg, SrcReg)) {
5253
BuildMI(MBB, MI, DL, get(AVR::MOVWRdRr), DestReg)
5354
.addReg(SrcReg, getKillRegState(KillSrc));
@@ -57,11 +58,17 @@ void AVRInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
5758
TRI.splitReg(DestReg, DestLo, DestHi);
5859
TRI.splitReg(SrcReg, SrcLo, SrcHi);
5960

60-
// Copy each individual register with the `MOV` instruction.
61-
BuildMI(MBB, MI, DL, get(AVR::MOVRdRr), DestLo)
62-
.addReg(SrcLo, getKillRegState(KillSrc));
63-
BuildMI(MBB, MI, DL, get(AVR::MOVRdRr), DestHi)
64-
.addReg(SrcHi, getKillRegState(KillSrc));
61+
if (DestLo == SrcHi) {
62+
BuildMI(MBB, MI, DL, get(AVR::MOVRdRr), DestHi)
63+
.addReg(SrcHi, getKillRegState(KillSrc));
64+
BuildMI(MBB, MI, DL, get(AVR::MOVRdRr), DestLo)
65+
.addReg(SrcLo, getKillRegState(KillSrc));
66+
} else {
67+
BuildMI(MBB, MI, DL, get(AVR::MOVRdRr), DestLo)
68+
.addReg(SrcLo, getKillRegState(KillSrc));
69+
BuildMI(MBB, MI, DL, get(AVR::MOVRdRr), DestHi)
70+
.addReg(SrcHi, getKillRegState(KillSrc));
71+
}
6572
}
6673
} else {
6774
if (AVR::GPR8RegClass.contains(DestReg, SrcReg)) {

llvm/test/CodeGen/AVR/pseudo/COPY.mir

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
# RUN: llc -O0 %s -o - | FileCheck %s
2+
3+
--- |
4+
target triple = "avr--"
5+
6+
define void @test_copy_nonoverlapping() {
7+
entry:
8+
ret void
9+
}
10+
11+
define void @test_copy_overlapping() {
12+
entry:
13+
ret void
14+
}
15+
16+
declare void @foo(i16 %0)
17+
...
18+
19+
---
20+
name: test_copy_nonoverlapping
21+
tracksRegLiveness: true
22+
body: |
23+
bb.0.entry:
24+
liveins: $r25r24
25+
26+
; CHECK-LABEL: test_copy_nonoverlapping:
27+
; CHECK: mov r22, r24
28+
; CHECK-NEXT: mov r23, r25
29+
30+
$r23r22 = COPY $r25r24
31+
RCALLk @foo, implicit $r24r23
32+
...
33+
34+
---
35+
name: test_copy_overlapping
36+
tracksRegLiveness: true
37+
body: |
38+
bb.0.entry:
39+
liveins: $r24r23
40+
41+
; CHECK-LABEL: test_copy_overlapping:
42+
; CHECK: mov r25, r24
43+
; CHECK-NEXT: mov r24, r23
44+
45+
$r25r24 = COPY $r24r23
46+
RCALLk @foo, implicit $r25r24
47+
...
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
; RUN: llc < %s -march=avr | FileCheck %s
2+
3+
; The bug can be found here:
4+
; https://github.com/rust-lang/rust/issues/98167
5+
;
6+
; In this test, `extractvalue` + `call` generate a copy with overlapping
7+
; registers (`$r25r24 = COPY $r24r23`) that used to be expanded incorrectly.
8+
9+
define void @main() {
10+
; CHECK-LABEL: main:
11+
; CHECK: rcall foo
12+
; CHECK-NEXT: mov r25, r24
13+
; CHECK-NEXT: mov r24, r23
14+
; CHECK-NEXT: rcall bar
15+
%1 = call { i8, i16 } @foo()
16+
%2 = extractvalue { i8, i16 } %1, 1
17+
call void @bar(i16 %2)
18+
ret void
19+
}
20+
21+
declare { i8, i16 } @foo()
22+
declare void @bar(i16 %0)

0 commit comments

Comments
 (0)