<div dir="ltr">Ah! Of course. I'll go fix that.<div><br>Thanks, </div><div><br></div><div>-Nirav</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, May 25, 2017 at 2:55 PM, Friedman, Eli <span dir="ltr"><<a href="mailto:efriedma@codeaurora.org" target="_blank">efriedma@codeaurora.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 5/25/2017 9:10 AM, Nirav Davé wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hmm. Wrapping was why I had it signed (I was looking at a negative BaseOffset). I believe we want the signed interpretation for the constant here.<br>
<br>
That said, it looks like getConstant only accepts a uint64_t so we are implicitly casting. The internal ConstantInt the value is fed into does allow it to be interpreted as signed. Presumably we do not have signed constant nodes to improve constant sharing. I wonder if it's worth propagating signedness throughout to explicitly avoid any overflow/underflow problems.<br>
</blockquote>
<br></span>
The overflow/underflow problem here is that if you build LLVM with -fsanitize=undefined, it will crash here trying to fold the addition in certain cases.  Hence the suggestion to use uint64_t, which wraps the same way ISD::ADD does.<br>
<br>
(We don't have signed/unsigned integer constants because we don't have signed/unsigned integer types in IR.)<div class="HOEnZb"><div class="h5"><br>
<br>
-Eli<br>
<br>
-- <br>
Employee of Qualcomm Innovation Center, Inc.<br>
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project<br>
<br>
</div></div></blockquote></div><br></div>