Skip to content

Commit

Permalink
[Aarch64] Correct register class for pseudo instructions
Browse files Browse the repository at this point in the history
This constrains the Mov* and similar pseudo instruction to take
GPR64common register classes rather than GPR64. GPR64 includs XZR
which is invalid here, because this pseudo instructions expands
into an adrp/add pair sharing a destination register. XZR is invalid
on add and attempting to encode it will instead increment the stack
pointer causing crashes (downstream report at [1]). The test case
there reproduces on LLVM11, but I do not have a test case that
reaches this code path on main, since it is being masked by
improved dead code elimination introduced in D91513. Nevertheless,
this seems like a good thing to fix in case there are other cases
that dead code elimination doesn't clean up (e.g. if `optnone` is
used and the optimization is skipped).

I think it would be worth auditing uses of GPR64 in pseudo
instructions to see if there are any similar issues, but I do not
have a high enough view of the backend or knowledge of the
Aarch64 architecture to do this quickly.

[1] JuliaLang/julia#39818

Reviewed By: t.p.northover

Differential Revision: https://reviews.llvm.org/D97435

(cherry picked from commit e20f69f)
  • Loading branch information
vtjnash authored and tstellar committed Nov 10, 2021
1 parent 60e114a commit ec1420d
Show file tree
Hide file tree
Showing 11 changed files with 39 additions and 37 deletions.
1 change: 1 addition & 0 deletions llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,7 @@ bool AArch64ExpandPseudo::expandMI(MachineBasicBlock &MBB,
case AArch64::MOVaddrEXT: {
// Expand into ADRP + ADD.
Register DstReg = MI.getOperand(0).getReg();
assert(DstReg != AArch64::XZR);
MachineInstrBuilder MIB1 =
BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(AArch64::ADRP), DstReg)
.add(MI.getOperand(1));
Expand Down
32 changes: 16 additions & 16 deletions llvm/lib/Target/AArch64/AArch64InstrInfo.td
Original file line number Diff line number Diff line change
Expand Up @@ -673,49 +673,49 @@ let isReMaterializable = 1, isCodeGenOnly = 1 in {
// removed, along with the AArch64Wrapper node.

let AddedComplexity = 10 in
def LOADgot : Pseudo<(outs GPR64:$dst), (ins i64imm:$addr),
[(set GPR64:$dst, (AArch64LOADgot tglobaladdr:$addr))]>,
def LOADgot : Pseudo<(outs GPR64common:$dst), (ins i64imm:$addr),
[(set GPR64common:$dst, (AArch64LOADgot tglobaladdr:$addr))]>,
Sched<[WriteLDAdr]>;

// The MOVaddr instruction should match only when the add is not folded
// into a load or store address.
def MOVaddr
: Pseudo<(outs GPR64:$dst), (ins i64imm:$hi, i64imm:$low),
[(set GPR64:$dst, (AArch64addlow (AArch64adrp tglobaladdr:$hi),
: Pseudo<(outs GPR64common:$dst), (ins i64imm:$hi, i64imm:$low),
[(set GPR64common:$dst, (AArch64addlow (AArch64adrp tglobaladdr:$hi),
tglobaladdr:$low))]>,
Sched<[WriteAdrAdr]>;
def MOVaddrJT
: Pseudo<(outs GPR64:$dst), (ins i64imm:$hi, i64imm:$low),
[(set GPR64:$dst, (AArch64addlow (AArch64adrp tjumptable:$hi),
: Pseudo<(outs GPR64common:$dst), (ins i64imm:$hi, i64imm:$low),
[(set GPR64common:$dst, (AArch64addlow (AArch64adrp tjumptable:$hi),
tjumptable:$low))]>,
Sched<[WriteAdrAdr]>;
def MOVaddrCP
: Pseudo<(outs GPR64:$dst), (ins i64imm:$hi, i64imm:$low),
[(set GPR64:$dst, (AArch64addlow (AArch64adrp tconstpool:$hi),
: Pseudo<(outs GPR64common:$dst), (ins i64imm:$hi, i64imm:$low),
[(set GPR64common:$dst, (AArch64addlow (AArch64adrp tconstpool:$hi),
tconstpool:$low))]>,
Sched<[WriteAdrAdr]>;
def MOVaddrBA
: Pseudo<(outs GPR64:$dst), (ins i64imm:$hi, i64imm:$low),
[(set GPR64:$dst, (AArch64addlow (AArch64adrp tblockaddress:$hi),
: Pseudo<(outs GPR64common:$dst), (ins i64imm:$hi, i64imm:$low),
[(set GPR64common:$dst, (AArch64addlow (AArch64adrp tblockaddress:$hi),
tblockaddress:$low))]>,
Sched<[WriteAdrAdr]>;
def MOVaddrTLS
: Pseudo<(outs GPR64:$dst), (ins i64imm:$hi, i64imm:$low),
[(set GPR64:$dst, (AArch64addlow (AArch64adrp tglobaltlsaddr:$hi),
: Pseudo<(outs GPR64common:$dst), (ins i64imm:$hi, i64imm:$low),
[(set GPR64common:$dst, (AArch64addlow (AArch64adrp tglobaltlsaddr:$hi),
tglobaltlsaddr:$low))]>,
Sched<[WriteAdrAdr]>;
def MOVaddrEXT
: Pseudo<(outs GPR64:$dst), (ins i64imm:$hi, i64imm:$low),
[(set GPR64:$dst, (AArch64addlow (AArch64adrp texternalsym:$hi),
: Pseudo<(outs GPR64common:$dst), (ins i64imm:$hi, i64imm:$low),
[(set GPR64common:$dst, (AArch64addlow (AArch64adrp texternalsym:$hi),
texternalsym:$low))]>,
Sched<[WriteAdrAdr]>;
// Normally AArch64addlow either gets folded into a following ldr/str,
// or together with an adrp into MOVaddr above. For cases with TLS, it
// might appear without either of them, so allow lowering it into a plain
// add.
def ADDlowTLS
: Pseudo<(outs GPR64:$dst), (ins GPR64:$src, i64imm:$low),
[(set GPR64:$dst, (AArch64addlow GPR64:$src,
: Pseudo<(outs GPR64sp:$dst), (ins GPR64sp:$src, i64imm:$low),
[(set GPR64sp:$dst, (AArch64addlow GPR64sp:$src,
tglobaltlsaddr:$low))]>,
Sched<[WriteAdr]>;

Expand Down
4 changes: 2 additions & 2 deletions llvm/test/CodeGen/AArch64/GlobalISel/select-add-low.mir
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ body: |
liveins: $x0
; CHECK-LABEL: name: select_add_low_without_offset
; CHECK: liveins: $x0
; CHECK: %add_low:gpr64 = MOVaddr target-flags(aarch64-page) @x, target-flags(aarch64-pageoff, aarch64-nc) @x
; CHECK: %add_low:gpr64common = MOVaddr target-flags(aarch64-page) @x, target-flags(aarch64-pageoff, aarch64-nc) @x
; CHECK: $x0 = COPY %add_low
; CHECK: RET_ReallyLR implicit $x0
%copy:gpr(p0) = COPY $x0
Expand All @@ -40,7 +40,7 @@ body: |
liveins: $x0
; CHECK-LABEL: name: select_add_low_with_offset
; CHECK: liveins: $x0
; CHECK: %add_low:gpr64 = MOVaddr target-flags(aarch64-page) @x + 1, target-flags(aarch64-pageoff, aarch64-nc) @x + 1
; CHECK: %add_low:gpr64common = MOVaddr target-flags(aarch64-page) @x + 1, target-flags(aarch64-pageoff, aarch64-nc) @x + 1
; CHECK: $x0 = COPY %add_low
; CHECK: RET_ReallyLR implicit $x0
%copy:gpr(p0) = COPY $x0
Expand Down
5 changes: 3 additions & 2 deletions llvm/test/CodeGen/AArch64/GlobalISel/select-blockaddress.mir
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ registers:
body: |
; CHECK-LABEL: name: test_blockaddress
; CHECK: bb.0 (%ir-block.0):
; CHECK: [[MOVaddrBA:%[0-9]+]]:gpr64 = MOVaddrBA target-flags(aarch64-page) blockaddress(@test_blockaddress, %ir-block.block), target-flags(aarch64-pageoff, aarch64-nc) blockaddress(@test_blockaddress, %ir-block.block)
; CHECK: [[MOVaddrBA:%[0-9]+]]:gpr64common = MOVaddrBA target-flags(aarch64-page) blockaddress(@test_blockaddress, %ir-block.block), target-flags(aarch64-pageoff, aarch64-nc) blockaddress(@test_blockaddress, %ir-block.block)
; CHECK: [[MOVaddr:%[0-9]+]]:gpr64common = MOVaddr target-flags(aarch64-page) @addr, target-flags(aarch64-pageoff, aarch64-nc) @addr
; CHECK: STRXui [[MOVaddrBA]], [[MOVaddr]], 0 :: (store (p0) into @addr)
; CHECK: [[COPY:%[0-9]+]]:gpr64 = COPY [[MOVaddrBA]]
; CHECK: STRXui [[COPY]], [[MOVaddr]], 0 :: (store (p0) into @addr)
; CHECK: BR [[MOVaddrBA]]
; CHECK: bb.1.block (address-taken):
; CHECK: RET_ReallyLR
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ body: |
; LARGE: RET_ReallyLR implicit $x0
; SMALL-LABEL: name: select_gv_with_offset
; SMALL: liveins: $x0
; SMALL: %g:gpr64 = MOVaddr target-flags(aarch64-page) @g + 1, target-flags(aarch64-pageoff, aarch64-nc) @g + 1
; SMALL: %g:gpr64common = MOVaddr target-flags(aarch64-page) @g + 1, target-flags(aarch64-pageoff, aarch64-nc) @g + 1
; SMALL: $x0 = COPY %g
; SMALL: RET_ReallyLR implicit $x0
; TINY-LABEL: name: select_gv_with_offset
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ body: |
; CHECK: Bcc 8, %bb.3, implicit $nzcv
; CHECK: bb.1:
; CHECK: successors: %bb.2(0x40000000), %bb.3(0x40000000)
; CHECK: [[MOVaddrJT:%[0-9]+]]:gpr64 = MOVaddrJT target-flags(aarch64-page) %jump-table.0, target-flags(aarch64-pageoff, aarch64-nc) %jump-table.0
; CHECK: [[MOVaddrJT:%[0-9]+]]:gpr64common = MOVaddrJT target-flags(aarch64-page) %jump-table.0, target-flags(aarch64-pageoff, aarch64-nc) %jump-table.0
; CHECK: early-clobber %6:gpr64, early-clobber %7:gpr64sp = JumpTableDest32 [[MOVaddrJT]], [[SUBREG_TO_REG]], %jump-table.0
; CHECK: BR %6
; CHECK: bb.2:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ body: |
; CHECK: bb.1.entry:
; CHECK: successors: %bb.3(0x2aaaaaab), %bb.4(0x2aaaaaab), %bb.2(0x2aaaaaab)
; CHECK: [[COPY2:%[0-9]+]]:gpr32 = COPY $wzr
; CHECK: [[MOVaddrJT:%[0-9]+]]:gpr64 = MOVaddrJT target-flags(aarch64-page) %jump-table.0, target-flags(aarch64-pageoff, aarch64-nc) %jump-table.0
; CHECK: [[MOVaddrJT:%[0-9]+]]:gpr64common = MOVaddrJT target-flags(aarch64-page) %jump-table.0, target-flags(aarch64-pageoff, aarch64-nc) %jump-table.0
; CHECK: early-clobber %18:gpr64, early-clobber %19:gpr64sp = JumpTableDest32 [[MOVaddrJT]], [[SUBREG_TO_REG]], %jump-table.0
; CHECK: BR %18
; CHECK: bb.2.sw.bb:
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/CodeGen/AArch64/GlobalISel/select-static.mir
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ registers:
- { id: 0, class: gpr }

# CHECK: body:
# LINUX-DEFAULT: %0:gpr64 = MOVaddr target-flags(aarch64-page) @var_local, target-flags(aarch64-pageoff, aarch64-nc) @var_local
# LINUX-DEFAULT: %0:gpr64common = MOVaddr target-flags(aarch64-page) @var_local, target-flags(aarch64-pageoff, aarch64-nc) @var_local
body: |
bb.0:
%0(p0) = G_GLOBAL_VALUE @var_local
Expand All @@ -91,7 +91,7 @@ registers:
- { id: 0, class: gpr }

# CHECK: body:
# LINUX-DEFAULT: %0:gpr64 = MOVaddr target-flags(aarch64-page) @var_got, target-flags(aarch64-pageoff, aarch64-nc) @var_got
# LINUX-DEFAULT: %0:gpr64common = MOVaddr target-flags(aarch64-page) @var_got, target-flags(aarch64-pageoff, aarch64-nc) @var_got
body: |
bb.0:
%0(p0) = G_GLOBAL_VALUE @var_got
Expand Down
8 changes: 4 additions & 4 deletions llvm/test/CodeGen/AArch64/GlobalISel/select.mir
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ registers:
- { id: 0, class: gpr }

# CHECK: body:
# IOS: %0:gpr64 = MOVaddr target-flags(aarch64-page) @var_local, target-flags(aarch64-pageoff, aarch64-nc) @var_local
# LINUX-PIC: %0:gpr64 = LOADgot target-flags(aarch64-got) @var_local
# IOS: %0:gpr64common = MOVaddr target-flags(aarch64-page) @var_local, target-flags(aarch64-pageoff, aarch64-nc) @var_local
# LINUX-PIC: %0:gpr64common = LOADgot target-flags(aarch64-got) @var_local
body: |
bb.0:
%0(p0) = G_GLOBAL_VALUE @var_local
Expand All @@ -93,8 +93,8 @@ registers:
- { id: 0, class: gpr }

# CHECK: body:
# IOS: %0:gpr64 = LOADgot target-flags(aarch64-got) @var_got
# LINUX-PIC: %0:gpr64 = LOADgot target-flags(aarch64-got) @var_got
# IOS: %0:gpr64common = LOADgot target-flags(aarch64-got) @var_got
# LINUX-PIC: %0:gpr64common = LOADgot target-flags(aarch64-got) @var_got
body: |
bb.0:
%0(p0) = G_GLOBAL_VALUE @var_got
Expand Down
10 changes: 5 additions & 5 deletions llvm/test/CodeGen/AArch64/elim-dead-mi.mir
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,18 @@
name: main
tracksRegLiveness: true
registers:
- { id: 0, class: gpr64, preferred-register: '' }
- { id: 0, class: gpr64common, preferred-register: '' }
- { id: 1, class: gpr64common, preferred-register: '' }
- { id: 2, class: gpr64, preferred-register: '' }
- { id: 3, class: gpr64common, preferred-register: '' }
- { id: 4, class: gpr32, preferred-register: '' }
- { id: 5, class: gpr32all, preferred-register: '' }
- { id: 6, class: gpr64, preferred-register: '' }
- { id: 6, class: gpr64common, preferred-register: '' }
body: |
bb.0:
successors: %bb.4(0x30000000), %bb.5(0x50000000)
%0:gpr64 = MOVaddr target-flags(aarch64-page) @c, target-flags(aarch64-pageoff, aarch64-nc) @c
%0:gpr64common = MOVaddr target-flags(aarch64-page) @c, target-flags(aarch64-pageoff, aarch64-nc) @c
CBZX killed %0, %bb.4
B %bb.5
Expand Down Expand Up @@ -55,7 +55,7 @@ body: |
bb.5:
successors: %bb.1(0x80000000)
; CHECK: bb.5
; CHECK-NOT: %6:gpr64 = MOVaddr target-flags(aarch64-page) @c, target-flags(aarch64-pageoff, aarch64-nc) @c
%6:gpr64 = MOVaddr target-flags(aarch64-page) @c, target-flags(aarch64-pageoff, aarch64-nc) @c
; CHECK-NOT: %6:gpr64common = MOVaddr target-flags(aarch64-page) @c, target-flags(aarch64-pageoff, aarch64-nc) @c
%6:gpr64common = MOVaddr target-flags(aarch64-page) @c, target-flags(aarch64-pageoff, aarch64-nc) @c
B %bb.1
...
6 changes: 3 additions & 3 deletions llvm/test/CodeGen/AArch64/loop-sink.mir
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ registers:
- { id: 17, class: gpr32, preferred-register: '' }
- { id: 18, class: gpr32sp, preferred-register: '' }
- { id: 19, class: gpr32, preferred-register: '' }
- { id: 20, class: gpr64, preferred-register: '' }
- { id: 20, class: gpr64common, preferred-register: '' }
- { id: 21, class: gpr64, preferred-register: '' }
- { id: 22, class: gpr64sp, preferred-register: '' }
- { id: 23, class: gpr64sp, preferred-register: '' }
Expand Down Expand Up @@ -338,7 +338,7 @@ body: |
; CHECK: [[COPY6:%[0-9]+]]:gpr64all = COPY [[ADDXri4]]
; CHECK: [[ADDXri5:%[0-9]+]]:gpr64sp = ADDXri [[COPY1]], 1, 0
; CHECK: [[COPY7:%[0-9]+]]:gpr64all = COPY [[ADDXri5]]
; CHECK: [[MOVaddrJT:%[0-9]+]]:gpr64 = MOVaddrJT target-flags(aarch64-page) %jump-table.0, target-flags(aarch64-pageoff, aarch64-nc) %jump-table.0
; CHECK: [[MOVaddrJT:%[0-9]+]]:gpr64common = MOVaddrJT target-flags(aarch64-page) %jump-table.0, target-flags(aarch64-pageoff, aarch64-nc) %jump-table.0
; CHECK: bb.1..backedge:
; CHECK: successors: %bb.9(0x09249249), %bb.2(0x76db6db7)
; CHECK: [[PHI:%[0-9]+]]:gpr64sp = PHI [[COPY7]], %bb.0, %7, %bb.9
Expand Down Expand Up @@ -415,7 +415,7 @@ body: |
%4:gpr64all = COPY %14
%15:gpr64sp = ADDXri %8, 1, 0
%5:gpr64all = COPY %15
%20:gpr64 = MOVaddrJT target-flags(aarch64-page) %jump-table.0, target-flags(aarch64-pageoff, aarch64-nc) %jump-table.0
%20:gpr64common = MOVaddrJT target-flags(aarch64-page) %jump-table.0, target-flags(aarch64-pageoff, aarch64-nc) %jump-table.0
bb.1..backedge:
successors: %bb.8(0x09249249), %bb.9(0x76db6db7)
Expand Down

0 comments on commit ec1420d

Please sign in to comment.