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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/GISelKnownBits.h:81
+protected:
+  virtual unsigned getMaxDepth() { return 6; }
+};
----------------
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 


================
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;
----------------
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 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65698