[PATCH] D138082: GlobalISel: ComputeNumSignBits from load range metadata

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp:633
+  if (TyBits != CR.getBitWidth())
+    return 1;
+
----------------
foad wrote:
> Can this just go in the `default` case above?
This was to defend against the vector case, TyBits is the scalar size


================
Comment at: llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp:692
+    GLoad *Ld = cast<GLoad>(&MI);
+    if (DemandedElts != 1)
+      break;
----------------
foad wrote:
> What exactly is this check for?
You're only looking for element 0 if it's a vector result type


================
Comment at: llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp:630
+  if (TyBits != CR.getBitWidth())
+    return 1;
+
----------------
foad wrote:
> arsenm wrote:
> > foad wrote:
> > > How could we hit this?
> > The anyextload case (or if the metadata was on a casted to vector result type)
> I don't understand the "casted to vector" case - is there a test case for that?
It's the case where every load is legalized to an s32 (which I'd like to stop doing eventually). Apparently it's not reachable in the globalisel version since we don't handle the extract_vector_elt look through yet


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138082/new/

https://reviews.llvm.org/D138082