[llvm] [GISel] Combine out-of-range shifts with value to 0 or -1 (PR #123510)
via llvm-commits
llvm-commits at lists.llvm.org
Sun Jan 19 06:20:45 PST 2025
https://github.com/lialan updated https://github.com/llvm/llvm-project/pull/123510
>From 63c7775f88cb90796ad2b69c44ce756591cb3545 Mon Sep 17 00:00:00 2001
From: Alan Li <me at alanli.org>
Date: Sun, 19 Jan 2025 12:42:52 +0800
Subject: [PATCH 1/4] First commit
---
.../llvm/CodeGen/GlobalISel/CombinerHelper.h | 28 +++++------
.../include/llvm/Target/GlobalISel/Combine.td | 13 +++--
.../lib/CodeGen/GlobalISel/CombinerHelper.cpp | 49 ++++++++++++++++++-
3 files changed, 70 insertions(+), 20 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
index 94e36e412b0cf7..da558de5946559 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
@@ -115,18 +115,13 @@ class CombinerHelper {
public:
CombinerHelper(GISelChangeObserver &Observer, MachineIRBuilder &B,
- bool IsPreLegalize,
- GISelKnownBits *KB = nullptr,
+ bool IsPreLegalize, GISelKnownBits *KB = nullptr,
MachineDominatorTree *MDT = nullptr,
const LegalizerInfo *LI = nullptr);
- GISelKnownBits *getKnownBits() const {
- return KB;
- }
+ GISelKnownBits *getKnownBits() const { return KB; }
- MachineIRBuilder &getBuilder() const {
- return Builder;
- }
+ MachineIRBuilder &getBuilder() const { return Builder; }
const TargetLowering &getTargetLowering() const;
@@ -150,8 +145,10 @@ class CombinerHelper {
/// is a legal integer constant type on the target.
bool isConstantLegalOrBeforeLegalizer(const LLT Ty) const;
- /// MachineRegisterInfo::replaceRegWith() and inform the observer of the changes
- void replaceRegWith(MachineRegisterInfo &MRI, Register FromReg, Register ToReg) const;
+ /// MachineRegisterInfo::replaceRegWith() and inform the observer of the
+ /// changes
+ void replaceRegWith(MachineRegisterInfo &MRI, Register FromReg,
+ Register ToReg) const;
/// Replace a single register operand with a new register and inform the
/// observer of the changes.
@@ -482,12 +479,12 @@ class CombinerHelper {
bool matchEqualDefs(const MachineOperand &MOP1,
const MachineOperand &MOP2) const;
- /// Return true if \p MOP is defined by a G_CONSTANT or splat with a value equal to
- /// \p C.
+ /// Return true if \p MOP is defined by a G_CONSTANT or splat with a value
+ /// equal to \p C.
bool matchConstantOp(const MachineOperand &MOP, int64_t C) const;
- /// Return true if \p MOP is defined by a G_FCONSTANT or splat with a value exactly
- /// equal to \p C.
+ /// Return true if \p MOP is defined by a G_FCONSTANT or splat with a value
+ /// exactly equal to \p C.
bool matchConstantFPOp(const MachineOperand &MOP, double C) const;
/// @brief Checks if constant at \p ConstIdx is larger than \p MI 's bitwidth
@@ -841,7 +838,8 @@ class CombinerHelper {
BuildFnTy &MatchInfo) const;
/// Match shifts greater or equal to the bitwidth of the operation.
- bool matchShiftsTooBig(MachineInstr &MI) const;
+ bool matchShiftsTooBig(MachineInstr &MI,
+ std::optional<int64_t> &MatchInfo) const;
/// Match constant LHS ops that should be commuted.
bool matchCommuteConstantToRHS(MachineInstr &MI) const;
diff --git a/llvm/include/llvm/Target/GlobalISel/Combine.td b/llvm/include/llvm/Target/GlobalISel/Combine.td
index 8641eabbdd84c6..ae0856550d9356 100644
--- a/llvm/include/llvm/Target/GlobalISel/Combine.td
+++ b/llvm/include/llvm/Target/GlobalISel/Combine.td
@@ -306,11 +306,18 @@ def ptr_add_immed_chain : GICombineRule<
[{ return Helper.matchPtrAddImmedChain(*${d}, ${matchinfo}); }]),
(apply [{ Helper.applyPtrAddImmedChain(*${d}, ${matchinfo}); }])>;
+def shift_result_matchdata : GIDefMatchData<"std::optional<int64_t>">;
def shifts_too_big : GICombineRule<
- (defs root:$root),
+ (defs root:$root, shift_result_matchdata:$matchinfo),
(match (wip_match_opcode G_SHL, G_ASHR, G_LSHR):$root,
- [{ return Helper.matchShiftsTooBig(*${root}); }]),
- (apply [{ Helper.replaceInstWithUndef(*${root}); }])>;
+ [{ return Helper.matchShiftsTooBig(*${root}, ${matchinfo}); }]),
+ (apply [{
+ if (${matchinfo}) {
+ Helper.replaceInstWithConstant(*${root}, *${matchinfo});
+ } else {
+ Helper.replaceInstWithUndef(*${root});
+ }
+ }])>;
// Fold shift (shift base x), y -> shift base, (x+y), if shifts are same
def shift_immed_matchdata : GIDefMatchData<"RegisterImmPair">;
diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index 4e3aaf5da7198c..6c04337ad73e0b 100644
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -35,6 +35,7 @@
#include "llvm/Support/Casting.h"
#include "llvm/Support/DivisionByConstantInfo.h"
#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/KnownBits.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Target/TargetMachine.h"
#include <cmath>
@@ -6590,12 +6591,56 @@ bool CombinerHelper::matchRedundantBinOpInEquality(MachineInstr &MI,
return CmpInst::isEquality(Pred) && Y.isValid();
}
-bool CombinerHelper::matchShiftsTooBig(MachineInstr &MI) const {
+static std::optional<unsigned>
+getMaxUsefulShift(KnownBits ValueKB, unsigned Opcode,
+ std::optional<int64_t> &Result) {
+ assert(Opcode == TargetOpcode::G_SHL || Opcode == TargetOpcode::G_LSHR ||
+ Opcode == TargetOpcode::G_ASHR && "Expect G_SHL, G_LSHR or G_ASHR.");
+ auto SignificantBits = 0;
+ switch (Opcode) {
+ case TargetOpcode::G_SHL:
+ SignificantBits = ValueKB.countMinTrailingZeros();
+ Result = 0;
+ break;
+ case TargetOpcode::G_LSHR:
+ Result = 0;
+ SignificantBits = ValueKB.countMinLeadingZeros();
+ break;
+ case TargetOpcode::G_ASHR:
+ if (ValueKB.isNonNegative()) {
+ SignificantBits = ValueKB.countMinLeadingZeros();
+ Result = 0;
+ } else if (ValueKB.isNegative()) {
+ SignificantBits = ValueKB.countMinLeadingOnes();
+ Result = -1;
+ } else {
+ // Cannot determine shift result.
+ Result = std::nullopt;
+ return false;
+ }
+ break;
+ default:
+ break;
+ }
+ return ValueKB.getBitWidth() - SignificantBits;
+}
+
+bool CombinerHelper::matchShiftsTooBig(
+ MachineInstr &MI, std::optional<int64_t> &MatchInfo) const {
+ Register ShiftVal = MI.getOperand(1).getReg();
Register ShiftReg = MI.getOperand(2).getReg();
LLT ResTy = MRI.getType(MI.getOperand(0).getReg());
auto IsShiftTooBig = [&](const Constant *C) {
auto *CI = dyn_cast<ConstantInt>(C);
- return CI && CI->uge(ResTy.getScalarSizeInBits());
+ if (!CI)
+ return false;
+ if (CI->uge(ResTy.getScalarSizeInBits())) {
+ MatchInfo = std::nullopt;
+ return true;
+ }
+ auto OptMaxUsefulShift = getMaxUsefulShift(KB->getKnownBits(ShiftVal),
+ MI.getOpcode(), MatchInfo);
+ return OptMaxUsefulShift && CI->uge(*OptMaxUsefulShift);
};
return matchUnaryPredicate(MRI, ShiftReg, IsShiftTooBig);
}
>From 56dfed9473140716806cfb41ed7371d5cb1e0030 Mon Sep 17 00:00:00 2001
From: Alan Li <me at alanli.org>
Date: Sun, 19 Jan 2025 20:29:57 +0800
Subject: [PATCH 2/4] Fix up
---
.../llvm/CodeGen/GlobalISel/CombinerHelper.h | 28 +++++++++++--------
.../lib/CodeGen/GlobalISel/CombinerHelper.cpp | 1 -
2 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
index da558de5946559..a51aa876e1deb0 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
@@ -115,13 +115,18 @@ class CombinerHelper {
public:
CombinerHelper(GISelChangeObserver &Observer, MachineIRBuilder &B,
- bool IsPreLegalize, GISelKnownBits *KB = nullptr,
+ bool IsPreLegalize,
+ GISelKnownBits *KB = nullptr,
MachineDominatorTree *MDT = nullptr,
const LegalizerInfo *LI = nullptr);
- GISelKnownBits *getKnownBits() const { return KB; }
+ GISelKnownBits *getKnownBits() const {
+ return KB;
+ }
- MachineIRBuilder &getBuilder() const { return Builder; }
+ MachineIRBuilder &getBuilder() const {
+ return Builder;
+ }
const TargetLowering &getTargetLowering() const;
@@ -145,10 +150,8 @@ class CombinerHelper {
/// is a legal integer constant type on the target.
bool isConstantLegalOrBeforeLegalizer(const LLT Ty) const;
- /// MachineRegisterInfo::replaceRegWith() and inform the observer of the
- /// changes
- void replaceRegWith(MachineRegisterInfo &MRI, Register FromReg,
- Register ToReg) const;
+ /// MachineRegisterInfo::replaceRegWith() and inform the observer of the changes
+ void replaceRegWith(MachineRegisterInfo &MRI, Register FromReg, Register ToReg) const;
/// Replace a single register operand with a new register and inform the
/// observer of the changes.
@@ -479,12 +482,12 @@ class CombinerHelper {
bool matchEqualDefs(const MachineOperand &MOP1,
const MachineOperand &MOP2) const;
- /// Return true if \p MOP is defined by a G_CONSTANT or splat with a value
- /// equal to \p C.
+ /// Return true if \p MOP is defined by a G_CONSTANT or splat with a value equal to
+ /// \p C.
bool matchConstantOp(const MachineOperand &MOP, int64_t C) const;
- /// Return true if \p MOP is defined by a G_FCONSTANT or splat with a value
- /// exactly equal to \p C.
+ /// Return true if \p MOP is defined by a G_FCONSTANT or splat with a value exactly
+ /// equal to \p C.
bool matchConstantFPOp(const MachineOperand &MOP, double C) const;
/// @brief Checks if constant at \p ConstIdx is larger than \p MI 's bitwidth
@@ -837,7 +840,8 @@ class CombinerHelper {
bool matchRedundantBinOpInEquality(MachineInstr &MI,
BuildFnTy &MatchInfo) const;
- /// Match shifts greater or equal to the bitwidth of the operation.
+ /// Match shifts greater or equal to the range (bitwidth of the operation, or
+ /// the source value).
bool matchShiftsTooBig(MachineInstr &MI,
std::optional<int64_t> &MatchInfo) const;
diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index 6c04337ad73e0b..b4cf1655b04728 100644
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -35,7 +35,6 @@
#include "llvm/Support/Casting.h"
#include "llvm/Support/DivisionByConstantInfo.h"
#include "llvm/Support/ErrorHandling.h"
-#include "llvm/Support/KnownBits.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Target/TargetMachine.h"
#include <cmath>
>From ce31b1f5be7b6f36033e539e03b4d2d44972151d Mon Sep 17 00:00:00 2001
From: Alan Li <me at alanli.org>
Date: Sun, 19 Jan 2025 22:14:39 +0800
Subject: [PATCH 3/4] adding AMDGPU tests
---
.../AMDGPU/GlobalISel/combine-shifts.mir | 114 ++++++++++++++++++
1 file changed, 114 insertions(+)
create mode 100644 llvm/test/CodeGen/AMDGPU/GlobalISel/combine-shifts.mir
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/combine-shifts.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/combine-shifts.mir
new file mode 100644
index 00000000000000..157ab5d7bb03dd
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/combine-shifts.mir
@@ -0,0 +1,114 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=amdgcn -run-pass=amdgpu-prelegalizer-combiner -verify-machineinstrs %s -o - | FileCheck %s
+
+---
+name: combine_ashr
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $vgpr0, $vgpr1, $vgpr31
+
+ liveins: $vgpr0, $vgpr1
+
+ ; CHECK-LABEL: name: combine_ashr
+ ; CHECK: liveins: $vgpr0, $vgpr1, $vgpr31, $vgpr0, $vgpr1
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1
+ ; CHECK-NEXT: [[MV:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY]](s32), [[COPY1]](s32)
+ ; CHECK-NEXT: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 0
+ ; CHECK-NEXT: G_STORE [[C]](s32), [[MV]](p0) :: (store (s32))
+ ; CHECK-NEXT: SI_RETURN
+ %9:_(s32) = COPY $vgpr0
+ %10:_(s32) = COPY $vgpr1
+ %0:_(p0) = G_MERGE_VALUES %9(s32), %10(s32)
+ %12:_(s32) = G_CONSTANT i32 10
+ %11:_(s32) = G_INTRINSIC intrinsic(@llvm.amdgcn.workitem.id.x)
+ %13:_(s32) = G_ASHR %11, %12(s32)
+ G_STORE %13(s32), %0(p0) :: (store (s32))
+ SI_RETURN
+
+...
+---
+name: combine_lshr
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $vgpr0, $vgpr1, $vgpr31
+
+ liveins: $vgpr0, $vgpr1
+
+ ; CHECK-LABEL: name: combine_lshr
+ ; CHECK: liveins: $vgpr0, $vgpr1, $vgpr31, $vgpr0, $vgpr1
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1
+ ; CHECK-NEXT: [[MV:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY]](s32), [[COPY1]](s32)
+ ; CHECK-NEXT: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 0
+ ; CHECK-NEXT: G_STORE [[C]](s32), [[MV]](p0) :: (store (s32))
+ ; CHECK-NEXT: SI_RETURN
+ %9:_(s32) = COPY $vgpr0
+ %10:_(s32) = COPY $vgpr1
+ %0:_(p0) = G_MERGE_VALUES %9(s32), %10(s32)
+ %12:_(s32) = G_CONSTANT i32 10
+ %11:_(s32) = G_INTRINSIC intrinsic(@llvm.amdgcn.workitem.id.x)
+ %13:_(s32) = G_LSHR %11, %12(s32)
+ G_STORE %13(s32), %0(p0) :: (store (s32))
+ SI_RETURN
+
+...
+---
+name: combine_shl
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $vgpr0, $vgpr1, $vgpr31
+
+ liveins: $vgpr0, $vgpr1
+
+ ; CHECK-LABEL: name: combine_shl
+ ; CHECK: liveins: $vgpr0, $vgpr1, $vgpr31, $vgpr0, $vgpr1
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1
+ ; CHECK-NEXT: [[MV:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY]](s32), [[COPY1]](s32)
+ ; CHECK-NEXT: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 0
+ ; CHECK-NEXT: G_STORE [[C]](s32), [[MV]](p0) :: (store (s32))
+ ; CHECK-NEXT: SI_RETURN
+ %9:_(s32) = COPY $vgpr0
+ %10:_(s32) = COPY $vgpr1
+ %0:_(p0) = G_MERGE_VALUES %9(s32), %10(s32)
+ %12:_(s32) = G_CONSTANT i32 16
+ %11:_(s32) = G_CONSTANT i32 4294901760
+ %13:_(s32) = G_SHL %11, %12(s32)
+ G_STORE %13(s32), %0(p0) :: (store (s32))
+ SI_RETURN
+
+...
+---
+name: combine_ashr2
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $vgpr0, $vgpr1, $vgpr31
+
+ liveins: $vgpr0, $vgpr1
+
+ ; CHECK-LABEL: name: combine_ashr2
+ ; CHECK: liveins: $vgpr0, $vgpr1, $vgpr31, $vgpr0, $vgpr1
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1
+ ; CHECK-NEXT: [[MV:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY]](s32), [[COPY1]](s32)
+ ; CHECK-NEXT: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 0
+ ; CHECK-NEXT: G_STORE [[C]](s32), [[MV]](p0) :: (store (s32))
+ ; CHECK-NEXT: SI_RETURN
+ %9:_(s32) = COPY $vgpr0
+ %10:_(s32) = COPY $vgpr1
+ %0:_(p0) = G_MERGE_VALUES %9(s32), %10(s32)
+ %12:_(s32) = G_CONSTANT i32 16
+ %11:_(s8) = G_CONSTANT i8 -126
+ G_STORE %13(s8), %0(p0) :: (store (s8))
+ SI_RETURN
+
+...
>From d0043990b2e248559a3ee38a735225ae532152d5 Mon Sep 17 00:00:00 2001
From: Alan Li <me at alanli.org>
Date: Sun, 19 Jan 2025 22:20:23 +0800
Subject: [PATCH 4/4] Adding new tests
---
llvm/test/CodeGen/AMDGPU/GlobalISel/combine-shifts.mir | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/combine-shifts.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/combine-shifts.mir
index 157ab5d7bb03dd..d3a3827f35a18a 100644
--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/combine-shifts.mir
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/combine-shifts.mir
@@ -100,14 +100,15 @@ body: |
; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1
; CHECK-NEXT: [[MV:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY]](s32), [[COPY1]](s32)
- ; CHECK-NEXT: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 0
- ; CHECK-NEXT: G_STORE [[C]](s32), [[MV]](p0) :: (store (s32))
+ ; CHECK-NEXT: [[C:%[0-9]+]]:_(s8) = G_CONSTANT i8 -1
+ ; CHECK-NEXT: G_STORE [[C]](s8), [[MV]](p0) :: (store (s8))
; CHECK-NEXT: SI_RETURN
%9:_(s32) = COPY $vgpr0
%10:_(s32) = COPY $vgpr1
%0:_(p0) = G_MERGE_VALUES %9(s32), %10(s32)
- %12:_(s32) = G_CONSTANT i32 16
- %11:_(s8) = G_CONSTANT i8 -126
+ %12:_(s32) = G_CONSTANT i32 1
+ %11:_(s8) = G_CONSTANT i8 -2
+ %13:_(s8) = G_ASHR %11, %12(s32)
G_STORE %13(s8), %0(p0) :: (store (s8))
SI_RETURN
More information about the llvm-commits
mailing list