[GlobalISel] Move DemandedElt's APInt size assert after isValid() check

[dsandersllvm]

This prevents the assertion from wrongly triggering on invalid LLT's

[llvmbot]

@llvm/pr-subscribers-llvm-globalisel

Author: Daniel Sanders (dsandersllvm)

Changes

This prevents the assertion from wrongly triggering on invalid LLT's

---

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

1 Files Affected:

• (modified) llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp (+9-9)

diff --git a/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp b/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
index fefa8f2ea85942..6ae6d56f2ca51b 100644
--- a/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
@@ -147,6 +147,15 @@ void GISelKnownBits::computeKnownBitsImpl(Register R, KnownBits &Known,
   unsigned Opcode = MI.getOpcode();
   LLT DstTy = MRI.getType(R);
 
+  // Handle the case where this is called on a register that does not have a
+  // type constraint (i.e. it has a register class constraint instead). This is
+  // unlikely to occur except by looking through copies but it is possible for
+  // the initial register being queried to be in this state.
+  if (!DstTy.isValid()) {
+    Known = KnownBits();
+    return;
+  }
+
 #ifndef NDEBUG
   if (DstTy.isFixedVector()) {
     assert(
@@ -158,15 +167,6 @@ void GISelKnownBits::computeKnownBitsImpl(Register R, KnownBits &Known,
   }
 #endif
 
-  // Handle the case where this is called on a register that does not have a
-  // type constraint (i.e. it has a register class constraint instead). This is
-  // unlikely to occur except by looking through copies but it is possible for
-  // the initial register being queried to be in this state.
-  if (!DstTy.isValid()) {
-    Known = KnownBits();
-    return;
-  }
-
   unsigned BitWidth = DstTy.getScalarSizeInBits();
   auto CacheEntry = ComputeKnownBitsCache.find(R);
   if (CacheEntry != ComputeKnownBitsCache.end()) {

[arsenm]

These are not mutually exclusive. A register class constraint does not imply there is no LLT

[dsandersllvm]

That's true but I'm just moving the existing code in this PR so that we check the LLT is valid before asserting on properties of it. Do you want me to fix that in the same PR?

[arsenm]

Might as well fix the comment while you're moving it

[dsandersllvm]

#116083. I can merge that PR into this one if you want to do both at the same time

[arsenm]

lgtm but could use test

[dsandersllvm]

Thanks, the test that failed for me was TEST_F(AArch64GISelMITest, TestKnownBitsCstWithClass) in llvm/unittests/CodeGen/GlobalISel/KnownBitsTest.cpp but it only seems to fail in our downstream fork. I haven't quite figured out why yet but both versions have isValid() false while having different values internally to the LLT.