Skip to content

Commit 3d2650b

Browse files
asbtopperc
andauthored
[RISCV] Use addi rather than addiw for immediates materialised by lui+addi(w) pairs when possible (#141663)
The logic in RISCVMatInt would previously produce lui+addiw on RV64 whenever a 32-bit integer must be materialised and the Hi20 and Lo12 parts are non-zero. However, sometimes addi can be used equivalently (whenever the sign extension behaviour of addiw would be a no-op). This patch moves to using addiw only when necessary. Although there is absolutely no advantage in terms of compressibility or performance, this has the following advantages: * It's more consistent with logic used elsewhere in the backend. For instance, RISCVOptWInstrs will try to convert addiw to addi on the basis it reduces test diffs vs RV32. * This matches the lowering GCC does in its codegen path. Unlike LLVM, GCC seems to have different expansion logic for the assembler vs codegen. For codegen it will use lui+addi if possible, but expanding `li` in the assembler will always produces lui+addiw as LLVM did prior to this commit. As someone who has been looking at a lot of gcc vs clang diffs lately, reducing unnecessary divergence is of at least some value. * As the diff for fold-mem-offset.ll shows, we can fold memory offsets in more cases when addi is used. Memory offset folding could be taught to recognise when the addiw could be replaced with an addi, but that seems unnecessary when we can simply change the logic in RISCVMatInt. As pointed out by @topperc during review, making this change without modifying RISCVOptWInstrs risks introducing some cases where we fail to remove a sext.w that we removed before. I've incorporated a patch based on a suggestion from Craig that avoids it, and also adds appropriate RISCVOptWInstrs test cases. The initial patch description noted that the main motivation was to avoid unnecessary differences both for RV32/RV64 and when comparing GCC, but noted that very occasionally we see a benefit from memory offset folding kicking in when it didn't before. Looking at the dynamic instruction count difference for SPEC benchmarks targeting rva22u64 and it shows we actually get a meaningful ~4.3% reduction in dynamic icount for 519.lbm_r. Looking at the data more closely, the codegen difference is in `LBM_performStreamCollideTRT` which as a function accounts for ~98% for dynamically executed instructions and the codegen diffs appear to be a knock-on effect of the address merging reducing register pressure right from function entry (for instance, we get a big reduction in dynamically executed loads in that function). Below is the icount data (rva22u64 -O3, no LTO): ``` Benchmark Baseline This PR Diff (%) ============================================================ 500.perlbench_r 174116601991 174115795810 -0.00% 502.gcc_r 218903280858 218903215788 -0.00% 505.mcf_r 131208029185 131207692803 -0.00% 508.namd_r 217497594322 217497594297 -0.00% 510.parest_r 289314486153 289313577652 -0.00% 511.povray_r 30640531048 30640765701 0.00% 519.lbm_r 95897914862 91712688050 -4.36% 520.omnetpp_r 134641549722 134867015683 0.17% 523.xalancbmk_r 281462762992 281432092673 -0.01% 525.x264_r 379776121941 379535558210 -0.06% 526.blender_r 659736022025 659738387343 0.00% 531.deepsjeng_r 349122867552 349122867481 -0.00% 538.imagick_r 238558760552 238558753269 -0.00% 541.leela_r 406578560612 406385135260 -0.05% 544.nab_r 400997131674 400996765827 -0.00% 557.xz_r 130079522194 129945515709 -0.10% ``` The instcounting setup I use doesn't have good support for drilling down into functions from outside the linked executable (e.g. libc). The difference in omnetpp all seems to come from there, and does not reflect any degradation in codegen quality. I can confirm with the current version of the PR there is no change in the number of static sext.w across all the SPEC 2017 benchmarks (rva22u64 O3) Co-authored-by: Craig Topper <[email protected]>
1 parent d313c09 commit 3d2650b

File tree

153 files changed

+1926
-1985
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

153 files changed

+1926
-1985
lines changed

llvm/lib/Target/RISCV/MCTargetDesc/RISCVMatInt.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,15 @@ static void generateInstSeqImpl(int64_t Val, const MCSubtargetInfo &STI,
9494
Res.emplace_back(RISCV::LUI, Hi20);
9595

9696
if (Lo12 || Hi20 == 0) {
97-
unsigned AddiOpc = (IsRV64 && Hi20) ? RISCV::ADDIW : RISCV::ADDI;
97+
unsigned AddiOpc = RISCV::ADDI;
98+
if (IsRV64 && Hi20) {
99+
// Use ADDIW rather than ADDI only when necessary for correctness. As
100+
// noted in RISCVOptWInstrs, this helps reduce test differences vs
101+
// RV32 without being a pessimization.
102+
int64_t LuiRes = SignExtend64<32>(Hi20 << 12);
103+
if (!isInt<32>(LuiRes + Lo12))
104+
AddiOpc = RISCV::ADDIW;
105+
}
98106
Res.emplace_back(AddiOpc, Lo12);
99107
}
100108
return;

llvm/lib/Target/RISCV/RISCVOptWInstrs.cpp

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -591,14 +591,34 @@ static bool isSignExtendedW(Register SrcReg, const RISCVSubtarget &ST,
591591
return false;
592592
break;
593593

594+
case RISCV::ADDI: {
595+
if (MI->getOperand(1).isReg() && MI->getOperand(1).getReg().isVirtual()) {
596+
if (MachineInstr *SrcMI = MRI.getVRegDef(MI->getOperand(1).getReg())) {
597+
if (SrcMI->getOpcode() == RISCV::LUI &&
598+
SrcMI->getOperand(1).isImm()) {
599+
uint64_t Imm = SrcMI->getOperand(1).getImm();
600+
Imm = SignExtend64<32>(Imm << 12);
601+
Imm += (uint64_t)MI->getOperand(2).getImm();
602+
if (isInt<32>(Imm))
603+
continue;
604+
}
605+
}
606+
}
607+
608+
if (hasAllWUsers(*MI, ST, MRI)) {
609+
FixableDef.insert(MI);
610+
break;
611+
}
612+
return false;
613+
}
614+
594615
// With these opcode, we can "fix" them with the W-version
595616
// if we know all users of the result only rely on bits 31:0
596617
case RISCV::SLLI:
597618
// SLLIW reads the lowest 5 bits, while SLLI reads lowest 6 bits
598619
if (MI->getOperand(2).getImm() >= 32)
599620
return false;
600621
[[fallthrough]];
601-
case RISCV::ADDI:
602622
case RISCV::ADD:
603623
case RISCV::LD:
604624
case RISCV::LWU:

llvm/test/CodeGen/RISCV/GlobalISel/alu-roundtrip.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@ define i64 @subi_i64(i64 %a) {
468468
; RV64IM-LABEL: subi_i64:
469469
; RV64IM: # %bb.0: # %entry
470470
; RV64IM-NEXT: lui a1, 1048275
471-
; RV64IM-NEXT: addiw a1, a1, -1548
471+
; RV64IM-NEXT: addi a1, a1, -1548
472472
; RV64IM-NEXT: add a0, a0, a1
473473
; RV64IM-NEXT: ret
474474
entry:

llvm/test/CodeGen/RISCV/GlobalISel/bitmanip.ll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,8 @@ define i24 @bitreverse_i24(i24 %x) {
174174
; RV64-NEXT: slli a1, a0, 16
175175
; RV64-NEXT: lui a2, 4096
176176
; RV64-NEXT: lui a3, 1048335
177-
; RV64-NEXT: addiw a2, a2, -1
178-
; RV64-NEXT: addiw a3, a3, 240
177+
; RV64-NEXT: addi a2, a2, -1
178+
; RV64-NEXT: addi a3, a3, 240
179179
; RV64-NEXT: and a0, a0, a2
180180
; RV64-NEXT: srli a0, a0, 16
181181
; RV64-NEXT: or a0, a0, a1
@@ -184,15 +184,15 @@ define i24 @bitreverse_i24(i24 %x) {
184184
; RV64-NEXT: slli a0, a0, 4
185185
; RV64-NEXT: and a0, a0, a3
186186
; RV64-NEXT: lui a3, 1047757
187-
; RV64-NEXT: addiw a3, a3, -820
187+
; RV64-NEXT: addi a3, a3, -820
188188
; RV64-NEXT: srli a1, a1, 4
189189
; RV64-NEXT: or a0, a1, a0
190190
; RV64-NEXT: and a1, a3, a2
191191
; RV64-NEXT: and a1, a0, a1
192192
; RV64-NEXT: slli a0, a0, 2
193193
; RV64-NEXT: and a0, a0, a3
194194
; RV64-NEXT: lui a3, 1047211
195-
; RV64-NEXT: addiw a3, a3, -1366
195+
; RV64-NEXT: addi a3, a3, -1366
196196
; RV64-NEXT: and a2, a3, a2
197197
; RV64-NEXT: srli a1, a1, 2
198198
; RV64-NEXT: or a0, a1, a0

llvm/test/CodeGen/RISCV/GlobalISel/div-by-constant.ll

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ define i32 @udiv_constant_no_add(i32 %a) nounwind {
2525
; RV64IM-NEXT: slli a0, a0, 32
2626
; RV64IM-NEXT: lui a1, 205
2727
; RV64IM-NEXT: srli a0, a0, 32
28-
; RV64IM-NEXT: addiw a1, a1, -819
28+
; RV64IM-NEXT: addi a1, a1, -819
2929
; RV64IM-NEXT: slli a1, a1, 12
3030
; RV64IM-NEXT: addi a1, a1, -819
3131
; RV64IM-NEXT: mul a0, a0, a1
@@ -62,7 +62,7 @@ define i32 @udiv_constant_add(i32 %a) nounwind {
6262
; RV64IM: # %bb.0:
6363
; RV64IM-NEXT: lui a1, 149797
6464
; RV64IM-NEXT: slli a2, a0, 32
65-
; RV64IM-NEXT: addiw a1, a1, -1755
65+
; RV64IM-NEXT: addi a1, a1, -1755
6666
; RV64IM-NEXT: srli a2, a2, 32
6767
; RV64IM-NEXT: mul a1, a2, a1
6868
; RV64IM-NEXT: srli a1, a1, 32
@@ -75,7 +75,7 @@ define i32 @udiv_constant_add(i32 %a) nounwind {
7575
; RV64IMZB-LABEL: udiv_constant_add:
7676
; RV64IMZB: # %bb.0:
7777
; RV64IMZB-NEXT: lui a1, 149797
78-
; RV64IMZB-NEXT: addiw a1, a1, -1755
78+
; RV64IMZB-NEXT: addi a1, a1, -1755
7979
; RV64IMZB-NEXT: zext.w a2, a0
8080
; RV64IMZB-NEXT: mul a1, a2, a1
8181
; RV64IMZB-NEXT: srli a1, a1, 32
@@ -286,7 +286,7 @@ define i16 @udiv16_constant_no_add(i16 %a) nounwind {
286286
; RV64IM-NEXT: slli a0, a0, 48
287287
; RV64IM-NEXT: lui a1, 13
288288
; RV64IM-NEXT: srli a0, a0, 48
289-
; RV64IM-NEXT: addiw a1, a1, -819
289+
; RV64IM-NEXT: addi a1, a1, -819
290290
; RV64IM-NEXT: mul a0, a0, a1
291291
; RV64IM-NEXT: srli a0, a0, 18
292292
; RV64IM-NEXT: ret
@@ -295,7 +295,7 @@ define i16 @udiv16_constant_no_add(i16 %a) nounwind {
295295
; RV64IMZB: # %bb.0:
296296
; RV64IMZB-NEXT: zext.h a0, a0
297297
; RV64IMZB-NEXT: lui a1, 13
298-
; RV64IMZB-NEXT: addiw a1, a1, -819
298+
; RV64IMZB-NEXT: addi a1, a1, -819
299299
; RV64IMZB-NEXT: mul a0, a0, a1
300300
; RV64IMZB-NEXT: srli a0, a0, 18
301301
; RV64IMZB-NEXT: ret
@@ -340,8 +340,8 @@ define i16 @udiv16_constant_add(i16 %a) nounwind {
340340
; RV64IM: # %bb.0:
341341
; RV64IM-NEXT: lui a1, 2
342342
; RV64IM-NEXT: lui a2, 16
343-
; RV64IM-NEXT: addiw a1, a1, 1171
344-
; RV64IM-NEXT: addiw a2, a2, -1
343+
; RV64IM-NEXT: addi a1, a1, 1171
344+
; RV64IM-NEXT: addi a2, a2, -1
345345
; RV64IM-NEXT: and a3, a0, a2
346346
; RV64IM-NEXT: mul a1, a3, a1
347347
; RV64IM-NEXT: srli a1, a1, 16

llvm/test/CodeGen/RISCV/GlobalISel/float-intrinsics.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1002,7 +1002,7 @@ define i1 @fpclass(float %x) {
10021002
; RV64I-NEXT: lui a4, 2048
10031003
; RV64I-NEXT: lui a5, 520192
10041004
; RV64I-NEXT: srli a2, a2, 33
1005-
; RV64I-NEXT: addiw a6, a4, -1
1005+
; RV64I-NEXT: addi a6, a4, -1
10061006
; RV64I-NEXT: xor a0, a0, a2
10071007
; RV64I-NEXT: subw a3, a2, a3
10081008
; RV64I-NEXT: sltu a3, a3, a6

llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/constant64.mir

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,8 @@ body: |
159159
; CHECK: liveins: $x10
160160
; CHECK-NEXT: {{ $}}
161161
; CHECK-NEXT: [[LUI:%[0-9]+]]:gpr = LUI 524288
162-
; CHECK-NEXT: [[ADDIW:%[0-9]+]]:gpr = ADDIW [[LUI]], 648
163-
; CHECK-NEXT: $x10 = COPY [[ADDIW]]
162+
; CHECK-NEXT: [[ADDI:%[0-9]+]]:gpr = ADDI [[LUI]], 648
163+
; CHECK-NEXT: $x10 = COPY [[ADDI]]
164164
; CHECK-NEXT: PseudoRET implicit $x10
165165
%0:gprb(s64) = G_CONSTANT i64 -2147483000
166166
$x10 = COPY %0(s64)

llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/fp-constant-f16.mir

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,21 @@
11
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 3
22
# RUN: llc -mtriple=riscv32 -mattr=+zfh -run-pass=instruction-select \
3-
# RUN: -simplify-mir -verify-machineinstrs %s -o - | FileCheck %s --check-prefixes=CHECK,RV32
3+
# RUN: -simplify-mir -verify-machineinstrs %s -o - | FileCheck %s
44
# RUN: llc -mtriple=riscv64 -mattr=+zfh -run-pass=instruction-select \
5-
# RUN: -simplify-mir -verify-machineinstrs %s -o - | FileCheck %s --check-prefixes=CHECK,RV64
5+
# RUN: -simplify-mir -verify-machineinstrs %s -o - | FileCheck %s
66

77
---
88
name: half_imm
99
legalized: true
1010
regBankSelected: true
1111
body: |
1212
bb.1:
13-
; RV32-LABEL: name: half_imm
14-
; RV32: [[LUI:%[0-9]+]]:gpr = LUI 4
15-
; RV32-NEXT: [[ADDI:%[0-9]+]]:gpr = ADDI [[LUI]], 584
16-
; RV32-NEXT: [[FMV_H_X:%[0-9]+]]:fpr16 = FMV_H_X [[ADDI]]
17-
; RV32-NEXT: $f10_h = COPY [[FMV_H_X]]
18-
; RV32-NEXT: PseudoRET implicit $f10_h
19-
;
20-
; RV64-LABEL: name: half_imm
21-
; RV64: [[LUI:%[0-9]+]]:gpr = LUI 4
22-
; RV64-NEXT: [[ADDIW:%[0-9]+]]:gpr = ADDIW [[LUI]], 584
23-
; RV64-NEXT: [[FMV_H_X:%[0-9]+]]:fpr16 = FMV_H_X [[ADDIW]]
24-
; RV64-NEXT: $f10_h = COPY [[FMV_H_X]]
25-
; RV64-NEXT: PseudoRET implicit $f10_h
13+
; CHECK-LABEL: name: half_imm
14+
; CHECK: [[LUI:%[0-9]+]]:gpr = LUI 4
15+
; CHECK-NEXT: [[ADDI:%[0-9]+]]:gpr = ADDI [[LUI]], 584
16+
; CHECK-NEXT: [[FMV_H_X:%[0-9]+]]:fpr16 = FMV_H_X [[ADDI]]
17+
; CHECK-NEXT: $f10_h = COPY [[FMV_H_X]]
18+
; CHECK-NEXT: PseudoRET implicit $f10_h
2619
%0:fprb(s16) = G_FCONSTANT half 0xH4248
2720
$f10_h = COPY %0(s16)
2821
PseudoRET implicit $f10_h

llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/fp-constant.mir

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,12 @@ legalized: true
1010
regBankSelected: true
1111
body: |
1212
bb.1:
13-
; RV32-LABEL: name: float_imm
14-
; RV32: [[LUI:%[0-9]+]]:gpr = LUI 263313
15-
; RV32-NEXT: [[ADDI:%[0-9]+]]:gpr = ADDI [[LUI]], -37
16-
; RV32-NEXT: [[FMV_W_X:%[0-9]+]]:fpr32 = FMV_W_X [[ADDI]]
17-
; RV32-NEXT: $f10_f = COPY [[FMV_W_X]]
18-
; RV32-NEXT: PseudoRET implicit $f10_f
19-
;
20-
; RV64-LABEL: name: float_imm
21-
; RV64: [[LUI:%[0-9]+]]:gpr = LUI 263313
22-
; RV64-NEXT: [[ADDIW:%[0-9]+]]:gpr = ADDIW [[LUI]], -37
23-
; RV64-NEXT: [[FMV_W_X:%[0-9]+]]:fpr32 = FMV_W_X [[ADDIW]]
24-
; RV64-NEXT: $f10_f = COPY [[FMV_W_X]]
25-
; RV64-NEXT: PseudoRET implicit $f10_f
13+
; CHECK-LABEL: name: float_imm
14+
; CHECK: [[LUI:%[0-9]+]]:gpr = LUI 263313
15+
; CHECK-NEXT: [[ADDI:%[0-9]+]]:gpr = ADDI [[LUI]], -37
16+
; CHECK-NEXT: [[FMV_W_X:%[0-9]+]]:fpr32 = FMV_W_X [[ADDI]]
17+
; CHECK-NEXT: $f10_f = COPY [[FMV_W_X]]
18+
; CHECK-NEXT: PseudoRET implicit $f10_f
2619
%0:fprb(s32) = G_FCONSTANT float 0x400921FB60000000
2720
$f10_f = COPY %0(s32)
2821
PseudoRET implicit $f10_f
@@ -109,14 +102,14 @@ body: |
109102
;
110103
; RV64-LABEL: name: double_imm
111104
; RV64: [[LUI:%[0-9]+]]:gpr = LUI 512
112-
; RV64-NEXT: [[ADDIW:%[0-9]+]]:gpr = ADDIW [[LUI]], 1169
113-
; RV64-NEXT: [[SLLI:%[0-9]+]]:gpr = SLLI [[ADDIW]], 15
114-
; RV64-NEXT: [[ADDI:%[0-9]+]]:gpr = ADDI [[SLLI]], -299
115-
; RV64-NEXT: [[SLLI1:%[0-9]+]]:gpr = SLLI [[ADDI]], 14
116-
; RV64-NEXT: [[ADDI1:%[0-9]+]]:gpr = ADDI [[SLLI1]], 1091
117-
; RV64-NEXT: [[SLLI2:%[0-9]+]]:gpr = SLLI [[ADDI1]], 12
118-
; RV64-NEXT: [[ADDI2:%[0-9]+]]:gpr = ADDI [[SLLI2]], -744
119-
; RV64-NEXT: [[FMV_D_X:%[0-9]+]]:fpr64 = FMV_D_X [[ADDI2]]
105+
; RV64-NEXT: [[ADDI:%[0-9]+]]:gpr = ADDI [[LUI]], 1169
106+
; RV64-NEXT: [[SLLI:%[0-9]+]]:gpr = SLLI [[ADDI]], 15
107+
; RV64-NEXT: [[ADDI1:%[0-9]+]]:gpr = ADDI [[SLLI]], -299
108+
; RV64-NEXT: [[SLLI1:%[0-9]+]]:gpr = SLLI [[ADDI1]], 14
109+
; RV64-NEXT: [[ADDI2:%[0-9]+]]:gpr = ADDI [[SLLI1]], 1091
110+
; RV64-NEXT: [[SLLI2:%[0-9]+]]:gpr = SLLI [[ADDI2]], 12
111+
; RV64-NEXT: [[ADDI3:%[0-9]+]]:gpr = ADDI [[SLLI2]], -744
112+
; RV64-NEXT: [[FMV_D_X:%[0-9]+]]:fpr64 = FMV_D_X [[ADDI3]]
120113
; RV64-NEXT: $f10_d = COPY [[FMV_D_X]]
121114
; RV64-NEXT: PseudoRET implicit $f10_d
122115
%0:fprb(s64) = G_FCONSTANT double 0x400921FB54442D18

0 commit comments

Comments
 (0)