[PATCH] D65698: [GISel]: Add GISelKnownBits analysis

[Aditya Nandakumar via Phabricator via llvm-commits]

aditya_nandakumar marked 2 inline comments as done.
aditya_nandakumar added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/GISelKnownBits.h:81
+protected:
+  virtual unsigned getMaxDepth() { return 6; }
+};
----------------
arsenm wrote:
> aditya_nandakumar wrote:
> > arsenm wrote:
> > > I would expect this to be a pass parameter, not a virtual function 
> > On second thought, does this need to be configurable at all? I don't see why users would need different depth's for different invocations. Maybe it's best to get rid of this configurability for now and re-add it when necessary.
> > 
> The hardcoded depth of 6 always seemed pretty arbitrary. Having to subclass the pass just to change this is too much work, and there’s a non-0 cost to the virtual if it’s configured this way. I don’t particularly care if it’s really configurable though 
Fair enough - I've removed the configurability for now (and the virtual function).


================
Comment at: llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp:299
+  case TargetOpcode::G_PTRTOINT:
+    // Fall through and handle them the same as zext/trunc.
+    LLVM_FALLTHROUGH;
----------------
arsenm wrote:
> aditya_nandakumar wrote:
> > arsenm wrote:
> > > I thought these disallowed mismatched sizes
> > I'm not sure I completely follow. Could you please elaborate?
> I thought unlike the IR instruction, G_INTTOPTR/G_PTRTOINT mandate the integer size match the point size, but it’s possible I imagined this 
I was not aware of such a restriction and from my quick look around the code base, I don't see such a requirement.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65698