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

Charles Davis cdavis5x at gmail.com
Wed Feb 11 19:52:32 PST 2015


> On Feb 11, 2015, at 8:22 PM, Chandler Carruth <chandlerc at google.com> wrote:
> 
> 
> On Wed, Feb 11, 2015 at 7:12 PM, Charles Davis <cdavis5x at gmail.com <mailto:cdavis5x at gmail.com>> wrote:
> > On Oct 21, 2014, at 3:00 PM, Philip Reames <listmail at philipreames.com <mailto: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 <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.
Right, silly me.
> 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.
OK.

BTW, I wanted to use Phabricator, but I can’t right now. I usually sign in there with this Gmail address, but I wound up changing cell phone numbers, which is a problem since I use two-step authentication, and then I stupidly fried the only computer authorized to bypass that before I could go update it. I can still read and write email because I have app passwords; I just can’t sign in using a browser. So now, I’m waiting for the Gmail people to get back to me. Would you like me to get a plain old Phabricator account instead?

Chip

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150211/5b761520/attachment.html>


More information about the llvm-commits mailing list