<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Feb 11, 2015, at 8:22 PM, Chandler Carruth <<a href="mailto:chandlerc@google.com" class="">chandlerc@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Wed, Feb 11, 2015 at 7:12 PM, Charles Davis <span dir="ltr" class=""><<a href="mailto:cdavis5x@gmail.com" target="_blank" class="">cdavis5x@gmail.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">> On Oct 21, 2014, at 3:00 PM, Philip Reames <<a href="mailto:listmail@philipreames.com" class="">listmail@philipreames.com</a>> wrote:<br class="">
><br class="">
> Author: reames<br class="">
> Date: Tue Oct 21 16:00:03 2014<br class="">
> New Revision: 220324<br class="">
><br class="">
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=220324&view=rev" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=220324&view=rev</a><br class="">
> Log:<br class="">
> Preserve 'nonnull' when changing type of the load.<br class="">
><br class="">
> When changing the type of a load in Chandler's recent InstCombine changes, we can preserve the new 'nonnull' metadata.<br class="">
><br class="">
> 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.<br class="">
</span>Hi Philip and Chandler,<br class="">
<br class="">
Some recent changes to LLVM may have exposed a problem with this.<br class="">
<br class="">
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.<br class=""></blockquote><div class=""><br class=""></div><div class="">Yuck.</div><div class=""><br class=""></div><div class="">I think it's probably important that we don't completely lose this information. Instead, we should transform !nonnull into !range metadata.</div><div class=""><br class=""></div><div class="">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.</div></div></div></div></div></blockquote>Right, silly me.<br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""> And we need to add !range when the target type is an integer type.</div></div></div></div></div></blockquote></div><div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""> <br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="">
Shall I commit this right away? And should we also fix stores with !nonnull metadata?</blockquote></div><br class="">Maybe incorporate the above, and mail out a fresh patch? Sending it with Phabricator would help make it easy to review.</div></div>
</div></blockquote>OK.</div><div><br class=""></div>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?<div class=""><br class=""></div><div class="">Chip</div><div class=""><br class=""></div></body></html>