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

Mon Aug 5 10:36:59 PDT 2019

dsanders added inline comments.

================
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:
> > 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.
> 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

FWIW, I believed this to be the case too but our documentation is rather vague on the subject (we should fix that, Aditya: could you do that as a follow-up?). We have two type-indices but I can't tell if we did that just to allow p0/s64 combinations or if we did it for other reasons too.
The best evidence I can see one way or the other is that LLVM-IR doesn't have this restriction and we haven't said it's difference but that's pretty weak evidence.

Overall I think Aditya's probably right but I expect there's a lot of us who thought this restriction was there.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65698