[llvm] [RISCV] Use isCompatible when we need runtime VSETVLIInfo equality. NFC (PR #94340)
Luke Lau via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 13 19:09:22 PDT 2024
https://github.com/lukel97 updated https://github.com/llvm/llvm-project/pull/94340
>From 4045d2ce5896dcd76edec42d52ea53a1d7fa8c1a Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Mon, 3 Jun 2024 16:33:57 +0100
Subject: [PATCH 1/4] [RISCV] Use separate VSETVLIInfo check for VL/VTYPE
equality. NFC
In VSETVLIInfo we define == as structural equality, e.g. unknown == unknown and invalid == invalid. We need this to check if the information at a block's entry or exit has changed.
However I think we may have been conflating it with the notion that the state of VL and VTYPE are the same, which isn't the case for two unknowns, and potentially in an upcoming patch where we don't know the value of an AVL register (see #93796)
This patch introduces a new method for checking that the VL and VTYPE are the same and switches it over where it would make a difference.
This should be NFC for now since the two uses of == we're replacing either didn't compare two unknowns or checked for unknowns. But it's needed if we're to introduce the notion of an AVLReg with a nullptr ValNo, which will need this non-reflexive equality.
---
llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp | 27 +++++++++++++++++---
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
index 82358cdd45edc..9a0297c518aae 100644
--- a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
@@ -732,6 +732,19 @@ class VSETVLIInfo {
return hasCompatibleVTYPE(Used, Require);
}
+ bool isKnownSame(const VSETVLIInfo &Other) const {
+ if (!isValid() || !Other.isValid())
+ return false;
+ if (isUnknown() || Other.isUnknown())
+ return false;
+ if (SEWLMULRatioOnly || Other.SEWLMULRatioOnly)
+ return false;
+ return hasSameAVL(Other) && hasSameVTYPE(Other);
+ }
+
+ // Whether or not two VSETVLIInfos are the same. Note that this does
+ // not necessarily mean they have the same AVL or VTYPE. Use
+ // isCompatible or isKnownSame for that instead.
bool operator==(const VSETVLIInfo &Other) const {
// Uninitialized is only equal to another Uninitialized.
if (!isValid())
@@ -745,7 +758,13 @@ class VSETVLIInfo {
if (Other.isUnknown())
return isUnknown();
- if (!hasSameAVL(Other))
+ // If what we know about the AVL is different, then they aren't equal.
+ if (State != Other.State)
+ return false;
+ if (hasAVLReg() && !(getAVLReg() == Other.getAVLReg() &&
+ getAVLVNInfo() == Other.getAVLVNInfo()))
+ return false;
+ if (hasAVLImm() && getAVLImm() != Other.getAVLImm())
return false;
// If the SEWLMULRatioOnly bits are different, then they aren't equal.
@@ -780,7 +799,7 @@ class VSETVLIInfo {
return VSETVLIInfo::getUnknown();
// If we have an exact, match return this.
- if (*this == Other)
+ if (isKnownSame(Other))
return *this;
// Not an exact match, but maybe the AVL and VLMAX are the same. If so,
@@ -1375,7 +1394,7 @@ bool RISCVInsertVSETVLI::needVSETVLIPHI(const VSETVLIInfo &Require,
// We found a VSET(I)VLI make sure it matches the output of the
// predecessor block.
VSETVLIInfo DefInfo = getInfoForVSETVLI(*DefMI);
- if (DefInfo != PBBExit)
+ if (!DefInfo.isKnownSame(PBBExit))
return true;
// Require has the same VL as PBBExit, so if the exit from the
@@ -1412,7 +1431,7 @@ void RISCVInsertVSETVLI::emitVSETVLIs(MachineBasicBlock &MBB) {
uint64_t TSFlags = MI.getDesc().TSFlags;
if (RISCVII::hasSEWOp(TSFlags)) {
- if (PrevInfo != CurInfo) {
+ if (!PrevInfo.isKnownSame(CurInfo)) {
// If this is the first implicit state change, and the state change
// requested can be proven to produce the same register contents, we
// can skip emitting the actual state change and continue as if we
>From 8508825909676975bcb7b969b1cac279ccf06045 Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Tue, 11 Jun 2024 11:46:24 +0800
Subject: [PATCH 2/4] Rework to reuse isCompatible
---
llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp | 29 ++++++++------------
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
index 9a0297c518aae..d0b88a4efe7bb 100644
--- a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
@@ -249,6 +249,13 @@ struct DemandedFields {
VLZeroness = true;
}
+ static DemandedFields all() {
+ DemandedFields DF;
+ DF.demandVTYPE();
+ DF.demandVL();
+ return DF;
+ }
+
// Make this the result of demanding both the fields in this and B.
void doUnion(const DemandedFields &B) {
VLAny |= B.VLAny;
@@ -609,7 +616,7 @@ class VSETVLIInfo {
bool hasNonZeroAVL(const LiveIntervals *LIS) const {
if (hasAVLImm())
return getAVLImm() > 0;
- if (hasAVLReg()) {
+ if (hasAVLReg() && LIS) {
if (auto *DefMI = getAVLDefMI(LIS))
return isNonZeroLoadImmediate(*DefMI);
}
@@ -713,14 +720,12 @@ class VSETVLIInfo {
const LiveIntervals *LIS) const {
assert(isValid() && Require.isValid() &&
"Can't compare invalid VSETVLIInfos");
- assert(!Require.SEWLMULRatioOnly &&
- "Expected a valid VTYPE for instruction!");
// Nothing is compatible with Unknown.
if (isUnknown() || Require.isUnknown())
return false;
// If only our VLMAX ratio is valid, then this isn't compatible.
- if (SEWLMULRatioOnly)
+ if (SEWLMULRatioOnly || Require.SEWLMULRatioOnly)
return false;
if (Used.VLAny && !(hasSameAVL(Require) && hasSameVLMAX(Require)))
@@ -732,16 +737,6 @@ class VSETVLIInfo {
return hasCompatibleVTYPE(Used, Require);
}
- bool isKnownSame(const VSETVLIInfo &Other) const {
- if (!isValid() || !Other.isValid())
- return false;
- if (isUnknown() || Other.isUnknown())
- return false;
- if (SEWLMULRatioOnly || Other.SEWLMULRatioOnly)
- return false;
- return hasSameAVL(Other) && hasSameVTYPE(Other);
- }
-
// Whether or not two VSETVLIInfos are the same. Note that this does
// not necessarily mean they have the same AVL or VTYPE. Use
// isCompatible or isKnownSame for that instead.
@@ -799,7 +794,7 @@ class VSETVLIInfo {
return VSETVLIInfo::getUnknown();
// If we have an exact, match return this.
- if (isKnownSame(Other))
+ if (isCompatible(DemandedFields::all(), Other, nullptr))
return *this;
// Not an exact match, but maybe the AVL and VLMAX are the same. If so,
@@ -1394,7 +1389,7 @@ bool RISCVInsertVSETVLI::needVSETVLIPHI(const VSETVLIInfo &Require,
// We found a VSET(I)VLI make sure it matches the output of the
// predecessor block.
VSETVLIInfo DefInfo = getInfoForVSETVLI(*DefMI);
- if (!DefInfo.isKnownSame(PBBExit))
+ if (!DefInfo.isCompatible(DemandedFields::all(), PBBExit, nullptr))
return true;
// Require has the same VL as PBBExit, so if the exit from the
@@ -1431,7 +1426,7 @@ void RISCVInsertVSETVLI::emitVSETVLIs(MachineBasicBlock &MBB) {
uint64_t TSFlags = MI.getDesc().TSFlags;
if (RISCVII::hasSEWOp(TSFlags)) {
- if (!PrevInfo.isKnownSame(CurInfo)) {
+ if (!PrevInfo.isCompatible(DemandedFields::all(), CurInfo, nullptr)) {
// If this is the first implicit state change, and the state change
// requested can be proven to produce the same register contents, we
// can skip emitting the actual state change and continue as if we
>From 02f84af8dbef5289e31f6745bef83c67d2c6bc32 Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Fri, 14 Jun 2024 10:06:20 +0800
Subject: [PATCH 3/4] Only use isCompatible in emitVSETVLIs
This should be the only place where after removing LiveIntervals it will
make a difference.
Also don't bother checking for nullptr LIS in hasNonZeroAVL since we
shouldn't hit that path with a fully demanded DemandedFields.
---
llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
index d0b88a4efe7bb..2fc1433fa7735 100644
--- a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
@@ -616,7 +616,7 @@ class VSETVLIInfo {
bool hasNonZeroAVL(const LiveIntervals *LIS) const {
if (hasAVLImm())
return getAVLImm() > 0;
- if (hasAVLReg() && LIS) {
+ if (hasAVLReg()) {
if (auto *DefMI = getAVLDefMI(LIS))
return isNonZeroLoadImmediate(*DefMI);
}
@@ -753,13 +753,7 @@ class VSETVLIInfo {
if (Other.isUnknown())
return isUnknown();
- // If what we know about the AVL is different, then they aren't equal.
- if (State != Other.State)
- return false;
- if (hasAVLReg() && !(getAVLReg() == Other.getAVLReg() &&
- getAVLVNInfo() == Other.getAVLVNInfo()))
- return false;
- if (hasAVLImm() && getAVLImm() != Other.getAVLImm())
+ if (!hasSameAVL(Other))
return false;
// If the SEWLMULRatioOnly bits are different, then they aren't equal.
@@ -794,7 +788,7 @@ class VSETVLIInfo {
return VSETVLIInfo::getUnknown();
// If we have an exact, match return this.
- if (isCompatible(DemandedFields::all(), Other, nullptr))
+ if (*this == Other)
return *this;
// Not an exact match, but maybe the AVL and VLMAX are the same. If so,
@@ -1389,7 +1383,7 @@ bool RISCVInsertVSETVLI::needVSETVLIPHI(const VSETVLIInfo &Require,
// We found a VSET(I)VLI make sure it matches the output of the
// predecessor block.
VSETVLIInfo DefInfo = getInfoForVSETVLI(*DefMI);
- if (!DefInfo.isCompatible(DemandedFields::all(), PBBExit, nullptr))
+ if (DefInfo != PBBExit)
return true;
// Require has the same VL as PBBExit, so if the exit from the
@@ -1426,7 +1420,7 @@ void RISCVInsertVSETVLI::emitVSETVLIs(MachineBasicBlock &MBB) {
uint64_t TSFlags = MI.getDesc().TSFlags;
if (RISCVII::hasSEWOp(TSFlags)) {
- if (!PrevInfo.isCompatible(DemandedFields::all(), CurInfo, nullptr)) {
+ if (!PrevInfo.isCompatible(DemandedFields::all(), CurInfo, LIS)) {
// If this is the first implicit state change, and the state change
// requested can be proven to produce the same register contents, we
// can skip emitting the actual state change and continue as if we
>From 9e4b899c0e9fbee1cb728d428e051ed9e9fd30e3 Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Fri, 14 Jun 2024 10:08:47 +0800
Subject: [PATCH 4/4] Remove no longer needed comment
---
llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp | 3 ---
1 file changed, 3 deletions(-)
diff --git a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
index 2fc1433fa7735..e7c13f180f64e 100644
--- a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
@@ -737,9 +737,6 @@ class VSETVLIInfo {
return hasCompatibleVTYPE(Used, Require);
}
- // Whether or not two VSETVLIInfos are the same. Note that this does
- // not necessarily mean they have the same AVL or VTYPE. Use
- // isCompatible or isKnownSame for that instead.
bool operator==(const VSETVLIInfo &Other) const {
// Uninitialized is only equal to another Uninitialized.
if (!isValid())
More information about the llvm-commits
mailing list