[RISCV] Use isCompatible when we need runtime VSETVLIInfo equality. NFC

[lukel97]

In VSETVLIInfo we define == as lattice 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 switches over the use in emitVSETVLIs to use isCompatible with all fields demanded, since we need VL and VTYPE to be known equal at runtime rather than just the VSETVLIInfos to be the same.

This should be NFC for now we shouldn't reach here with an unknown state. 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. Specifically, hasSameAVL should return false for this case, but == should return true.

[llvmbot]

@llvm/pr-subscribers-backend-risc-v

Author: Luke Lau (lukel97)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/94340.diff

1 Files Affected:

• (modified) llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp (+23-4)

diff --git a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
index a96768240a933..6f7f58ca5206d 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

[BeMg]

LGTM

[lukel97]

I've tweaked this to reuse isCompatible instead of introducing a new method. == should now be strictly for lattice equality

[lukel97]

Turns out we shouldn't use it in intersect, as that still requires lattice equality. We also don't need it for needVSETVLIPHI since we bail out for the case without a VNInfo in #94686

[preames]

LGTM to the revised form.

Please make sure you remember to update the commit message when landing though!