[llvm] [GlobalISel] Correct comment about type vs register class (PR #116083)

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 15 16:32:39 PST 2024


https://github.com/dsandersllvm updated https://github.com/llvm/llvm-project/pull/116083

>From 4042038d00e80bbc94f63b21a976c341ba080aaa Mon Sep 17 00:00:00 2001
From: Daniel Sanders <daniel_l_sanders at apple.com>
Date: Tue, 12 Nov 2024 18:10:14 -0800
Subject: [PATCH 1/3] [GlobalISel] Move DemandedElt's APInt size assert after
 isValid() check

This prevents the assertion from wrongly triggering on invalid LLT's
---
 llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

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()) {

>From 4321b949b88ef69217f1d3f64d514efc78b09c54 Mon Sep 17 00:00:00 2001
From: Daniel Sanders <daniel_l_sanders at apple.com>
Date: Wed, 13 Nov 2024 09:35:48 -0800
Subject: [PATCH 2/3] [GlobalISel] Correct comment about type vs register class

Type and register class aren't mutually exclusive in gMIR but target
instructions don't have types on their operands that have register classes.
---
 llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp b/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
index 6ae6d56f2ca51b..179ac9deb3d4a5 100644
--- a/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
@@ -148,9 +148,10 @@ void GISelKnownBits::computeKnownBitsImpl(Register R, KnownBits &Known,
   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.
+  // type constraint (i.e. it's a target instruction with 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;

>From ec7cf1ce2a30c110e40b248b37c10361a8362885 Mon Sep 17 00:00:00 2001
From: Daniel Sanders <daniel_l_sanders at apple.com>
Date: Fri, 15 Nov 2024 16:32:21 -0800
Subject: [PATCH 3/3] fixup new comment to account for changes to behaviour
 since this was first written

---
 llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp b/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
index 179ac9deb3d4a5..e5477939d3eedc 100644
--- a/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp
@@ -148,10 +148,8 @@ void GISelKnownBits::computeKnownBitsImpl(Register R, KnownBits &Known,
   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's a target instruction with 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.
+  // type constraint. For example, it may be post-ISel or this target might not
+  // preserve the type when early-selecting instructions.
   if (!DstTy.isValid()) {
     Known = KnownBits();
     return;



More information about the llvm-commits mailing list