Skip to content

Commit 9e892ea

Browse files
vtjnashvchuravy
vtjnash
authored and
vchuravy
committed
AArch64 GIsel: legalize lshr operands, even if it is poison
Previously, this caused GlobalISel to emit invalid IR (a gpr32 to gpr64 copy) and fail during verification. While this shift is not defined (returns poison), it should not crash codegen, as it may appear inside dead code (for example, a select instruction), and it is legal IR input, as long as the value is unused. Discovered while trying to build Julia with LLVM v13: JuliaLang/julia#42602. Reviewed By: aemerson Differential Revision: https://reviews.llvm.org/D114389 (cherry picked from commit 18308e1)
1 parent 6ced34d commit 9e892ea

File tree

3 files changed

+92
-29
lines changed

3 files changed

+92
-29
lines changed

‎llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp

Lines changed: 22 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1908,35 +1908,6 @@ bool AArch64InstructionSelector::preISelLower(MachineInstr &I) {
19081908
MachineRegisterInfo &MRI = MF.getRegInfo();
19091909

19101910
switch (I.getOpcode()) {
1911-
case TargetOpcode::G_SHL:
1912-
case TargetOpcode::G_ASHR:
1913-
case TargetOpcode::G_LSHR: {
1914-
// These shifts are legalized to have 64 bit shift amounts because we want
1915-
// to take advantage of the existing imported selection patterns that assume
1916-
// the immediates are s64s. However, if the shifted type is 32 bits and for
1917-
// some reason we receive input GMIR that has an s64 shift amount that's not
1918-
// a G_CONSTANT, insert a truncate so that we can still select the s32
1919-
// register-register variant.
1920-
Register SrcReg = I.getOperand(1).getReg();
1921-
Register ShiftReg = I.getOperand(2).getReg();
1922-
const LLT ShiftTy = MRI.getType(ShiftReg);
1923-
const LLT SrcTy = MRI.getType(SrcReg);
1924-
if (SrcTy.isVector())
1925-
return false;
1926-
assert(!ShiftTy.isVector() && "unexpected vector shift ty");
1927-
if (SrcTy.getSizeInBits() != 32 || ShiftTy.getSizeInBits() != 64)
1928-
return false;
1929-
auto *AmtMI = MRI.getVRegDef(ShiftReg);
1930-
assert(AmtMI && "could not find a vreg definition for shift amount");
1931-
if (AmtMI->getOpcode() != TargetOpcode::G_CONSTANT) {
1932-
// Insert a subregister copy to implement a 64->32 trunc
1933-
auto Trunc = MIB.buildInstr(TargetOpcode::COPY, {SrcTy}, {})
1934-
.addReg(ShiftReg, 0, AArch64::sub_32);
1935-
MRI.setRegBank(Trunc.getReg(0), RBI.getRegBank(AArch64::GPRRegBankID));
1936-
I.getOperand(2).setReg(Trunc.getReg(0));
1937-
}
1938-
return true;
1939-
}
19401911
case TargetOpcode::G_STORE: {
19411912
bool Changed = contractCrossBankCopyIntoStore(I, MRI);
19421913
MachineOperand &SrcOp = I.getOperand(0);
@@ -2860,6 +2831,28 @@ bool AArch64InstructionSelector::select(MachineInstr &I) {
28602831
if (Opcode == TargetOpcode::G_SHL &&
28612832
MRI.getType(I.getOperand(0).getReg()).isVector())
28622833
return selectVectorSHL(I, MRI);
2834+
2835+
// These shifts were legalized to have 64 bit shift amounts because we
2836+
// want to take advantage of the selection patterns that assume the
2837+
// immediates are s64s, however, selectBinaryOp will assume both operands
2838+
// will have the same bit size.
2839+
{
2840+
Register SrcReg = I.getOperand(1).getReg();
2841+
Register ShiftReg = I.getOperand(2).getReg();
2842+
const LLT ShiftTy = MRI.getType(ShiftReg);
2843+
const LLT SrcTy = MRI.getType(SrcReg);
2844+
if (!SrcTy.isVector() && SrcTy.getSizeInBits() == 32 &&
2845+
ShiftTy.getSizeInBits() == 64) {
2846+
assert(!ShiftTy.isVector() && "unexpected vector shift ty");
2847+
auto *AmtMI = MRI.getVRegDef(ShiftReg);
2848+
assert(AmtMI && "could not find a vreg definition for shift amount");
2849+
// Insert a subregister copy to implement a 64->32 trunc
2850+
auto Trunc = MIB.buildInstr(TargetOpcode::COPY, {SrcTy}, {})
2851+
.addReg(ShiftReg, 0, AArch64::sub_32);
2852+
MRI.setRegBank(Trunc.getReg(0), RBI.getRegBank(AArch64::GPRRegBankID));
2853+
I.getOperand(2).setReg(Trunc.getReg(0));
2854+
}
2855+
}
28632856
LLVM_FALLTHROUGH;
28642857
case TargetOpcode::G_FADD:
28652858
case TargetOpcode::G_FSUB:

‎llvm/test/CodeGen/AArch64/GlobalISel/select-binop.mir

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,32 @@ body: |
491491
$w0 = COPY %2(s32)
492492
...
493493

494+
---
495+
name: shl_s32_64_gpr
496+
legalized: true
497+
regBankSelected: true
498+
499+
registers:
500+
- { id: 0, class: gpr }
501+
- { id: 1, class: gpr }
502+
- { id: 2, class: gpr }
503+
504+
body: |
505+
bb.0:
506+
liveins: $w0, $x1
507+
508+
; CHECK-LABEL: name: shl_s32_64_gpr
509+
; CHECK: [[COPY:%[0-9]+]]:gpr32 = COPY $w0
510+
; CHECK: [[COPY1:%[0-9]+]]:gpr64all = COPY $x1
511+
; CHECK: [[COPY2:%[0-9]+]]:gpr32 = COPY [[COPY1]].sub_32
512+
; CHECK: [[LSLVWr:%[0-9]+]]:gpr32 = LSLVWr [[COPY]], [[COPY2]]
513+
; CHECK: $w0 = COPY [[LSLVWr]]
514+
%0(s32) = COPY $w0
515+
%1(s64) = COPY $x1
516+
%2(s32) = G_SHL %0, %1
517+
$w0 = COPY %2(s32)
518+
...
519+
494520
---
495521
# Same as add_s64_gpr, for G_SHL operations.
496522
name: shl_s64_gpr

‎llvm/test/CodeGen/AArch64/GlobalISel/select-scalar-shift-imm.mir

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,50 @@ body: |
9999
$w0 = COPY %2(s32)
100100
RET_ReallyLR implicit $w0
101101
102+
...
103+
---
104+
name: ashr_cimm_32_64
105+
legalized: true
106+
regBankSelected: true
107+
body: |
108+
bb.1:
109+
liveins: $w0
110+
111+
; CHECK-LABEL: name: ashr_cimm_32_64
112+
; CHECK: [[COPY:%[0-9]+]]:gpr32 = COPY $w0
113+
; CHECK: [[MOVi64imm:%[0-9]+]]:gpr64 = MOVi64imm -8
114+
; CHECK: [[COPY1:%[0-9]+]]:gpr32 = COPY [[MOVi64imm]]
115+
; CHECK: [[ASRVWr:%[0-9]+]]:gpr32 = ASRVWr [[COPY]], [[COPY1]]
116+
; CHECK: $w0 = COPY [[ASRVWr]]
117+
; CHECK: RET_ReallyLR implicit $w0
118+
%0:gpr(s32) = COPY $w0
119+
%3:gpr(s64) = G_CONSTANT i64 -8
120+
%2:gpr(s32) = G_ASHR %0, %3(s64)
121+
$w0 = COPY %2(s32)
122+
RET_ReallyLR implicit $w0
123+
124+
...
125+
---
126+
name: lshr_cimm_32_64
127+
legalized: true
128+
regBankSelected: true
129+
body: |
130+
bb.1:
131+
liveins: $w0
132+
133+
; CHECK-LABEL: name: lshr_cimm_32_64
134+
; CHECK: [[COPY:%[0-9]+]]:gpr32 = COPY $w0
135+
; CHECK: [[MOVi64imm:%[0-9]+]]:gpr64 = MOVi64imm -8
136+
; CHECK: [[COPY1:%[0-9]+]]:gpr32 = COPY [[MOVi64imm]]
137+
; CHECK: [[LSRVWr:%[0-9]+]]:gpr32 = LSRVWr [[COPY]], [[COPY1]]
138+
; CHECK: $w0 = COPY [[LSRVWr]]
139+
; CHECK: RET_ReallyLR implicit $w0
140+
%0:gpr(s32) = COPY $w0
141+
%3:gpr(s64) = G_CONSTANT i64 -8
142+
%2:gpr(s32) = G_LSHR %0, %3(s64)
143+
$w0 = COPY %2(s32)
144+
RET_ReallyLR implicit $w0
145+
102146
...
103147
---
104148
name: ashr_cimm_64

0 commit comments

Comments
 (0)