[PATCH] D79234: [ValueTracking] Fix computeKnownBits() with bitwidth-changing ptrtoint

[Nikita Popov via Phabricator via llvm-commits]

nikic marked an inline comment as done.
nikic added a comment.

In D79234#2039337 <https://reviews.llvm.org/D79234#2039337>, @nikic wrote:

> In D79234#2023197 <https://reviews.llvm.org/D79234#2023197>, @efriedma wrote:
>
> > Historically, we allowed IR without a datalayout, so we didn't actually know what the pointer width would be when the IR was eventually compiled.  Given that, allowing arbitrary casts was the least-bad alternative: otherwise, we wouldn't be able to verify a module.
> >
> > Now that datalayout is required, we could maybe restrict the legal ptrtoint casts?  Not sure what we would do about ptrtoint constant expressions; constant expressions that don't reference globals aren't associated with a particular module.
>
>
> Thanks for the context! Would anything speak against making the validity of constant expressions dependent on which module they are used in? That is, the same constant expression could be valid in one module and invalid in another, depending on data layout. As the validator works by recursively checking constants referenced in the module, I think that should work.
>
> I may look into adding this requirement as a longer term change. I'd prefer to land this patch in the meantime though.


I poked at this a bit, and ran into the first snag in https://github.com/llvm/llvm-project/blob/a63eedd049bfde97f1d73caa61d27df600501c54/llvm/lib/IR/BasicBlock.cpp#L79. This is creating an inttoptr cast in the BasicBlock destructor, so we have no data layout to ensure the correct integer size is used. That may make it infeasible to enforce this restriction...



================
Comment at: lib/Analysis/ValueTracking.cpp:794
+        KnownBits RHSKnown =
+            computeKnownBits(A, Depth+1, Query(Q, I)).anyextOrTrunc(BitWidth);
         Known.Zero |= RHSKnown.Zero;
----------------
efriedma wrote:
> LangRef says ptrtoint zero-extends.
Here we have the knownbits of the ptrtoint result and want to reason backwards to the knownbits of the ptrtoint operand. If the ptrtoint zexted, that means we need to trunc. If the ptrtoint truncated, that means we need to anyext.

Here anyextOrTrunc is used as the inverse operation to zextOrTrunc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79234