Skip to content

[TTI][RISCV] Unconditionally break critical edges to sink ADDI #108889

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Nov 26, 2024
Merged

[TTI][RISCV] Unconditionally break critical edges to sink ADDI #108889

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3f4f26e
[TTI][RISCV] Unconditionally break critical edges to sink ADDI
preames Sep 16, 2024
4c878bb
Start fixing failed rebase
preames Sep 16, 2024
aeec393
test convert
preames Sep 16, 2024
4a89449
Address review comment
preames Sep 17, 2024
5fa0835
Merge branch 'main' into pr-riscv-machine-sink-addi
preames Sep 17, 2024
0617aac
Clang format
preames Sep 17, 2024
b06c457
Merge branch 'main' into pr-riscv-machine-sink-addi
preames Nov 25, 2024
bc21dfd
Merge branch 'main' into pr-riscv-machine-sink-addi
preames Nov 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions llvm/include/llvm/CodeGen/TargetInstrInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,12 @@ class TargetInstrInfo : public MCInstrInfo {
return true;
}

/// For a "cheap" instruction which doesn't enable additional sinking,
/// should MachineSink break a critical edge to sink it anyways?
virtual bool shouldBreakCriticalEdgeToSink(MachineInstr &MI) const {
return false;
}

protected:
/// For instructions with opcodes for which the M_REMATERIALIZABLE flag is
/// set, this hook lets the target specify whether the instruction is actually
Expand Down
4 changes: 3 additions & 1 deletion llvm/lib/CodeGen/MachineSink.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -952,7 +952,9 @@ bool MachineSinking::isWorthBreakingCriticalEdge(
}
}

return false;
// Let the target decide if it's worth breaking this
// critical edge for a "cheap" instruction.
return TII->shouldBreakCriticalEdgeToSink(MI);
}

bool MachineSinking::isLegalToBreakCriticalEdge(MachineInstr &MI,
Expand Down
4 changes: 4 additions & 0 deletions llvm/lib/Target/RISCV/RISCVInstrInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ class RISCVInstrInfo : public RISCVGenInstrInfo {

bool isReallyTriviallyReMaterializable(const MachineInstr &MI) const override;

bool shouldBreakCriticalEdgeToSink(MachineInstr &MI) const override {
return MI.getOpcode() == RISCV::ADDI;
}

void copyPhysRegVector(MachineBasicBlock &MBB,
MachineBasicBlock::iterator MBBI, const DebugLoc &DL,
MCRegister DstReg, MCRegister SrcReg, bool KillSrc,
Expand Down
14 changes: 8 additions & 6 deletions llvm/test/CodeGen/RISCV/aext-to-sext.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,14 @@ bar:
define i64 @sext_phi_constants(i32 signext %c) {
; RV64I-LABEL: sext_phi_constants:
; RV64I: # %bb.0:
; RV64I-NEXT: li a1, -1
; RV64I-NEXT: bnez a0, .LBB2_2
; RV64I-NEXT: # %bb.1: # %iffalse
; RV64I-NEXT: li a1, -2
; RV64I-NEXT: .LBB2_2: # %merge
; RV64I-NEXT: slli a0, a1, 32
; RV64I-NEXT: beqz a0, .LBB2_2
; RV64I-NEXT: # %bb.1:
; RV64I-NEXT: li a0, -1
; RV64I-NEXT: j .LBB2_3
; RV64I-NEXT: .LBB2_2: # %iffalse
; RV64I-NEXT: li a0, -2
; RV64I-NEXT: .LBB2_3: # %merge
; RV64I-NEXT: slli a0, a0, 32
; RV64I-NEXT: srli a0, a0, 32
; RV64I-NEXT: ret
%a = icmp ne i32 %c, 0
Expand Down
12 changes: 6 additions & 6 deletions llvm/test/CodeGen/RISCV/machine-sink-load-immediate.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,13 +184,13 @@ declare i32 @toupper()
define signext i32 @overlap_live_ranges(ptr %arg, i32 signext %arg1) {
; CHECK-LABEL: overlap_live_ranges:
; CHECK: # %bb.0: # %bb
; CHECK-NEXT: li a3, 1
; CHECK-NEXT: li a2, 13
; CHECK-NEXT: bne a1, a3, .LBB1_2
; CHECK-NEXT: li a2, 1
; CHECK-NEXT: bne a1, a2, .LBB1_2
; CHECK-NEXT: # %bb.1: # %bb2
; CHECK-NEXT: lw a2, 4(a0)
; CHECK-NEXT: .LBB1_2: # %bb5
; CHECK-NEXT: mv a0, a2
; CHECK-NEXT: lw a0, 4(a0)
; CHECK-NEXT: ret
; CHECK-NEXT: .LBB1_2:
; CHECK-NEXT: li a0, 13
; CHECK-NEXT: ret
bb:
%i = icmp eq i32 %arg1, 1
Expand Down
10 changes: 4 additions & 6 deletions llvm/test/CodeGen/RISCV/rv64m-w-insts-legalization.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,13 @@ define signext i32 @mulw(i32 signext %s, i32 signext %n, i32 signext %k) nounwin
; CHECK-LABEL: mulw:
; CHECK: # %bb.0: # %entry
; CHECK-NEXT: li a2, 1
; CHECK-NEXT: bge a0, a1, .LBB0_3
; CHECK-NEXT: # %bb.1: # %for.body.preheader
; CHECK-NEXT: li a2, 1
; CHECK-NEXT: .LBB0_2: # %for.body
; CHECK-NEXT: bge a0, a1, .LBB0_2
; CHECK-NEXT: .LBB0_1: # %for.body
; CHECK-NEXT: # =>This Inner Loop Header: Depth=1
; CHECK-NEXT: mulw a2, a0, a2
; CHECK-NEXT: addiw a0, a0, 1
; CHECK-NEXT: blt a0, a1, .LBB0_2
; CHECK-NEXT: .LBB0_3: # %for.cond.cleanup
; CHECK-NEXT: blt a0, a1, .LBB0_1
; CHECK-NEXT: .LBB0_2: # %for.cond.cleanup
; CHECK-NEXT: mv a0, a2
; CHECK-NEXT: ret
entry:
Expand Down
12 changes: 6 additions & 6 deletions llvm/test/CodeGen/RISCV/select-const.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,22 +61,22 @@ define signext i32 @select_const_int_pow2_zero(i1 zeroext %a) nounwind {
define signext i32 @select_const_int_harder(i1 zeroext %a) nounwind {
; RV32-LABEL: select_const_int_harder:
; RV32: # %bb.0:
; RV32-NEXT: mv a1, a0
; RV32-NEXT: li a0, 6
; RV32-NEXT: bnez a1, .LBB3_2
; RV32-NEXT: bnez a0, .LBB3_2
; RV32-NEXT: # %bb.1:
; RV32-NEXT: li a0, 38
; RV32-NEXT: ret
; RV32-NEXT: .LBB3_2:
; RV32-NEXT: li a0, 6
; RV32-NEXT: ret
;
; RV64-LABEL: select_const_int_harder:
; RV64: # %bb.0:
; RV64-NEXT: mv a1, a0
; RV64-NEXT: li a0, 6
; RV64-NEXT: bnez a1, .LBB3_2
; RV64-NEXT: bnez a0, .LBB3_2
; RV64-NEXT: # %bb.1:
; RV64-NEXT: li a0, 38
; RV64-NEXT: ret
; RV64-NEXT: .LBB3_2:
; RV64-NEXT: li a0, 6
Copy link
Collaborator Author

@preames preames Sep 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These select lowering changes are arguably real regressions. We are loosing the opportunity to allocate the untaken result into a register and then conditionally overwrite it. The tail duplication probably wouldn't happen in real code, so we'd end up emitting an extra unconditional jump to bypass the critical edge we split before the join.

Specifically for these selects in tests, most can be done via arithmetic expansions instead. I started to implement that, but ended up pulling on a few too many intertwined changes. I do plan to go back to this, but I don't consider the select impact blocking here.

; RV64-NEXT: ret
%1 = select i1 %a, i32 6, i32 38
ret i32 %1
Expand Down
48 changes: 24 additions & 24 deletions llvm/test/CodeGen/RISCV/select.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1585,22 +1585,22 @@ define i32 @select_cst_not5(i32 signext %a, i32 signext %b) {
define i32 @select_cst_unknown(i32 signext %a, i32 signext %b) {
; RV32IM-LABEL: select_cst_unknown:
; RV32IM: # %bb.0:
; RV32IM-NEXT: mv a2, a0
; RV32IM-NEXT: li a0, 5
; RV32IM-NEXT: blt a2, a1, .LBB42_2
; RV32IM-NEXT: blt a0, a1, .LBB42_2
; RV32IM-NEXT: # %bb.1:
; RV32IM-NEXT: li a0, -7
; RV32IM-NEXT: ret
; RV32IM-NEXT: .LBB42_2:
; RV32IM-NEXT: li a0, 5
; RV32IM-NEXT: ret
;
; RV64IM-LABEL: select_cst_unknown:
; RV64IM: # %bb.0:
; RV64IM-NEXT: mv a2, a0
; RV64IM-NEXT: li a0, 5
; RV64IM-NEXT: blt a2, a1, .LBB42_2
; RV64IM-NEXT: blt a0, a1, .LBB42_2
; RV64IM-NEXT: # %bb.1:
; RV64IM-NEXT: li a0, -7
; RV64IM-NEXT: ret
; RV64IM-NEXT: .LBB42_2:
; RV64IM-NEXT: li a0, 5
; RV64IM-NEXT: ret
;
; RV64IMXVTCONDOPS-LABEL: select_cst_unknown:
Expand All @@ -1626,22 +1626,22 @@ define i32 @select_cst_unknown(i32 signext %a, i32 signext %b) {
define i32 @select_cst1(i1 zeroext %cond) {
; RV32IM-LABEL: select_cst1:
; RV32IM: # %bb.0:
; RV32IM-NEXT: mv a1, a0
; RV32IM-NEXT: li a0, 10
; RV32IM-NEXT: bnez a1, .LBB43_2
; RV32IM-NEXT: bnez a0, .LBB43_2
; RV32IM-NEXT: # %bb.1:
; RV32IM-NEXT: li a0, 20
; RV32IM-NEXT: ret
; RV32IM-NEXT: .LBB43_2:
; RV32IM-NEXT: li a0, 10
; RV32IM-NEXT: ret
;
; RV64IM-LABEL: select_cst1:
; RV64IM: # %bb.0:
; RV64IM-NEXT: mv a1, a0
; RV64IM-NEXT: li a0, 10
; RV64IM-NEXT: bnez a1, .LBB43_2
; RV64IM-NEXT: bnez a0, .LBB43_2
; RV64IM-NEXT: # %bb.1:
; RV64IM-NEXT: li a0, 20
; RV64IM-NEXT: ret
; RV64IM-NEXT: .LBB43_2:
; RV64IM-NEXT: li a0, 10
; RV64IM-NEXT: ret
;
; RV64IMXVTCONDOPS-LABEL: select_cst1:
Expand All @@ -1664,24 +1664,24 @@ define i32 @select_cst1(i1 zeroext %cond) {
define i32 @select_cst2(i1 zeroext %cond) {
; RV32IM-LABEL: select_cst2:
; RV32IM: # %bb.0:
; RV32IM-NEXT: mv a1, a0
; RV32IM-NEXT: li a0, 10
; RV32IM-NEXT: bnez a1, .LBB44_2
; RV32IM-NEXT: bnez a0, .LBB44_2
; RV32IM-NEXT: # %bb.1:
; RV32IM-NEXT: lui a0, 5
; RV32IM-NEXT: addi a0, a0, -480
; RV32IM-NEXT: ret
; RV32IM-NEXT: .LBB44_2:
; RV32IM-NEXT: li a0, 10
; RV32IM-NEXT: ret
;
; RV64IM-LABEL: select_cst2:
; RV64IM: # %bb.0:
; RV64IM-NEXT: mv a1, a0
; RV64IM-NEXT: li a0, 10
; RV64IM-NEXT: bnez a1, .LBB44_2
; RV64IM-NEXT: bnez a0, .LBB44_2
; RV64IM-NEXT: # %bb.1:
; RV64IM-NEXT: lui a0, 5
; RV64IM-NEXT: addiw a0, a0, -480
; RV64IM-NEXT: ret
; RV64IM-NEXT: .LBB44_2:
; RV64IM-NEXT: li a0, 10
; RV64IM-NEXT: ret
;
; RV64IMXVTCONDOPS-LABEL: select_cst2:
Expand Down Expand Up @@ -1782,24 +1782,24 @@ define i32 @select_cst4(i1 zeroext %cond) {
define i32 @select_cst5(i1 zeroext %cond) {
; RV32IM-LABEL: select_cst5:
; RV32IM: # %bb.0:
; RV32IM-NEXT: mv a1, a0
; RV32IM-NEXT: li a0, 2047
; RV32IM-NEXT: bnez a1, .LBB47_2
; RV32IM-NEXT: bnez a0, .LBB47_2
; RV32IM-NEXT: # %bb.1:
; RV32IM-NEXT: lui a0, 1
; RV32IM-NEXT: addi a0, a0, -2047
; RV32IM-NEXT: ret
; RV32IM-NEXT: .LBB47_2:
; RV32IM-NEXT: li a0, 2047
; RV32IM-NEXT: ret
;
; RV64IM-LABEL: select_cst5:
; RV64IM: # %bb.0:
; RV64IM-NEXT: mv a1, a0
; RV64IM-NEXT: li a0, 2047
; RV64IM-NEXT: bnez a1, .LBB47_2
; RV64IM-NEXT: bnez a0, .LBB47_2
; RV64IM-NEXT: # %bb.1:
; RV64IM-NEXT: lui a0, 1
; RV64IM-NEXT: addiw a0, a0, -2047
; RV64IM-NEXT: ret
; RV64IM-NEXT: .LBB47_2:
; RV64IM-NEXT: li a0, 2047
; RV64IM-NEXT: ret
;
; RV64IMXVTCONDOPS-LABEL: select_cst5:
Expand Down
92 changes: 48 additions & 44 deletions llvm/test/CodeGen/RISCV/sextw-removal.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1032,82 +1032,86 @@ bb7: ; preds = %bb2
define signext i32 @bug(i32 signext %x) {
; CHECK-LABEL: bug:
; CHECK: # %bb.0: # %entry
; CHECK-NEXT: beqz a0, .LBB18_4
; CHECK-NEXT: beqz a0, .LBB18_5
; CHECK-NEXT: # %bb.1: # %if.end
; CHECK-NEXT: srliw a2, a0, 16
; CHECK-NEXT: seqz a1, a2
; CHECK-NEXT: slli a1, a1, 4
; CHECK-NEXT: sllw a1, a0, a1
; CHECK-NEXT: li a0, 16
; CHECK-NEXT: beqz a2, .LBB18_3
; CHECK-NEXT: srliw a1, a0, 16
; CHECK-NEXT: seqz a2, a1
; CHECK-NEXT: slli a2, a2, 4
; CHECK-NEXT: sllw a0, a0, a2
; CHECK-NEXT: beqz a1, .LBB18_3
; CHECK-NEXT: # %bb.2: # %if.end
; CHECK-NEXT: li a0, 32
; CHECK-NEXT: .LBB18_3: # %if.end
; CHECK-NEXT: srliw a2, a1, 24
; CHECK-NEXT: li a1, 32
; CHECK-NEXT: j .LBB18_4
; CHECK-NEXT: .LBB18_3:
; CHECK-NEXT: li a1, 16
; CHECK-NEXT: .LBB18_4: # %if.end
; CHECK-NEXT: srliw a2, a0, 24
; CHECK-NEXT: seqz a2, a2
; CHECK-NEXT: slli a3, a2, 3
; CHECK-NEXT: sllw a1, a1, a3
; CHECK-NEXT: sllw a0, a0, a3
; CHECK-NEXT: negw a2, a2
; CHECK-NEXT: andi a2, a2, -8
; CHECK-NEXT: add a0, a0, a2
; CHECK-NEXT: srliw a2, a1, 28
; CHECK-NEXT: add a1, a1, a2
; CHECK-NEXT: srliw a2, a0, 28
; CHECK-NEXT: seqz a2, a2
; CHECK-NEXT: slli a3, a2, 2
; CHECK-NEXT: sllw a1, a1, a3
; CHECK-NEXT: sllw a0, a0, a3
; CHECK-NEXT: negw a2, a2
; CHECK-NEXT: andi a2, a2, -4
; CHECK-NEXT: add a0, a0, a2
; CHECK-NEXT: srliw a2, a1, 30
; CHECK-NEXT: add a1, a1, a2
; CHECK-NEXT: srliw a2, a0, 30
; CHECK-NEXT: seqz a2, a2
; CHECK-NEXT: slli a3, a2, 1
; CHECK-NEXT: sllw a1, a1, a3
; CHECK-NEXT: sllw a0, a0, a3
; CHECK-NEXT: negw a2, a2
; CHECK-NEXT: andi a2, a2, -2
; CHECK-NEXT: add a0, a0, a2
; CHECK-NEXT: not a1, a1
; CHECK-NEXT: srli a1, a1, 31
; CHECK-NEXT: addw a0, a0, a1
; CHECK-NEXT: .LBB18_4: # %cleanup
; CHECK-NEXT: add a1, a1, a2
; CHECK-NEXT: not a0, a0
; CHECK-NEXT: srli a0, a0, 31
; CHECK-NEXT: addw a0, a1, a0
; CHECK-NEXT: .LBB18_5: # %cleanup
; CHECK-NEXT: ret
;
; NOREMOVAL-LABEL: bug:
; NOREMOVAL: # %bb.0: # %entry
; NOREMOVAL-NEXT: beqz a0, .LBB18_4
; NOREMOVAL-NEXT: beqz a0, .LBB18_5
; NOREMOVAL-NEXT: # %bb.1: # %if.end
; NOREMOVAL-NEXT: srliw a2, a0, 16
; NOREMOVAL-NEXT: seqz a1, a2
; NOREMOVAL-NEXT: slli a1, a1, 4
; NOREMOVAL-NEXT: sllw a1, a0, a1
; NOREMOVAL-NEXT: li a0, 16
; NOREMOVAL-NEXT: beqz a2, .LBB18_3
; NOREMOVAL-NEXT: srliw a1, a0, 16
; NOREMOVAL-NEXT: seqz a2, a1
; NOREMOVAL-NEXT: slli a2, a2, 4
; NOREMOVAL-NEXT: sllw a0, a0, a2
; NOREMOVAL-NEXT: beqz a1, .LBB18_3
; NOREMOVAL-NEXT: # %bb.2: # %if.end
; NOREMOVAL-NEXT: li a0, 32
; NOREMOVAL-NEXT: .LBB18_3: # %if.end
; NOREMOVAL-NEXT: srliw a2, a1, 24
; NOREMOVAL-NEXT: li a1, 32
; NOREMOVAL-NEXT: j .LBB18_4
; NOREMOVAL-NEXT: .LBB18_3:
; NOREMOVAL-NEXT: li a1, 16
; NOREMOVAL-NEXT: .LBB18_4: # %if.end
; NOREMOVAL-NEXT: srliw a2, a0, 24
; NOREMOVAL-NEXT: seqz a2, a2
; NOREMOVAL-NEXT: slli a3, a2, 3
; NOREMOVAL-NEXT: sllw a1, a1, a3
; NOREMOVAL-NEXT: sllw a0, a0, a3
; NOREMOVAL-NEXT: negw a2, a2
; NOREMOVAL-NEXT: andi a2, a2, -8
; NOREMOVAL-NEXT: add a0, a0, a2
; NOREMOVAL-NEXT: srliw a2, a1, 28
; NOREMOVAL-NEXT: add a1, a1, a2
; NOREMOVAL-NEXT: srliw a2, a0, 28
; NOREMOVAL-NEXT: seqz a2, a2
; NOREMOVAL-NEXT: slli a3, a2, 2
; NOREMOVAL-NEXT: sllw a1, a1, a3
; NOREMOVAL-NEXT: sllw a0, a0, a3
; NOREMOVAL-NEXT: negw a2, a2
; NOREMOVAL-NEXT: andi a2, a2, -4
; NOREMOVAL-NEXT: add a0, a0, a2
; NOREMOVAL-NEXT: srliw a2, a1, 30
; NOREMOVAL-NEXT: add a1, a1, a2
; NOREMOVAL-NEXT: srliw a2, a0, 30
; NOREMOVAL-NEXT: seqz a2, a2
; NOREMOVAL-NEXT: slli a3, a2, 1
; NOREMOVAL-NEXT: sllw a1, a1, a3
; NOREMOVAL-NEXT: sllw a0, a0, a3
; NOREMOVAL-NEXT: negw a2, a2
; NOREMOVAL-NEXT: andi a2, a2, -2
; NOREMOVAL-NEXT: add a0, a0, a2
; NOREMOVAL-NEXT: not a1, a1
; NOREMOVAL-NEXT: srli a1, a1, 31
; NOREMOVAL-NEXT: addw a0, a0, a1
; NOREMOVAL-NEXT: .LBB18_4: # %cleanup
; NOREMOVAL-NEXT: add a1, a1, a2
; NOREMOVAL-NEXT: not a0, a0
; NOREMOVAL-NEXT: srli a0, a0, 31
; NOREMOVAL-NEXT: addw a0, a1, a0
; NOREMOVAL-NEXT: .LBB18_5: # %cleanup
; NOREMOVAL-NEXT: ret
entry:
%tobool.not = icmp eq i32 %x, 0
Expand Down
Loading