[llvm] r220324 - Preserve 'nonnull' when changing type of the load.

[Chandler Carruth]

On Wed, Feb 11, 2015 at 7:12 PM, Charles Davis <cdavis5x at gmail.com> wrote:

> > On Oct 21, 2014, at 3:00 PM, Philip Reames <listmail at philipreames.com>
> wrote:
> >
> > Author: reames
> > Date: Tue Oct 21 16:00:03 2014
> > New Revision: 220324
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=220324&view=rev
> > Log:
> > Preserve 'nonnull' when changing type of the load.
> >
> > When changing the type of a load in Chandler's recent InstCombine
> changes, we can preserve the new 'nonnull' metadata.
> >
> > I considered adding an assert since 'nonnull' is only valid on pointer
> types, but casting a pointer to a non-pointer would involve more than a
> bitcast anyways.  If someone extends this transform to handle more than
> bitcasts, the verifier will report the malformed IR, so a separate
> assertion isn't needed.
> Hi Philip and Chandler,
>
> Some recent changes to LLVM may have exposed a problem with this.
>
> Under certain conditions, if InstCombine attempts to canonicalize loads
> like this, we get a load of an integer with !nonnull metadata. I’ve
> attached a patch that contains a test that demonstrates this, and a fix
> that makes the test pass.
>

Yuck.

I think it's probably important that we don't completely lose this
information. Instead, we should transform !nonnull into !range metadata.

Also, you don't want to check for an integer type -- that's not the
relevant thing. We need to remove !nonnull any time the target type is a
non-pointer type. And we need to add !range when the target type is an
integer type.


>
> Shall I commit this right away? And should we also fix stores with
> !nonnull metadata?


Maybe incorporate the above, and mail out a fresh patch? Sending it with
Phabricator would help make it easy to review.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150211/1ffc3366/attachment.html>