[PATCH] D65698: [GISel]: Add GISelKnownBits analysis
Aditya Nandakumar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Aug 4 17:19:40 PDT 2019
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
More information about the llvm-commits
mailing list