<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Oct 17, 2014 at 5:17 PM, Quentin Colombet <span dir="ltr"><<a href="mailto:qcolombet@apple.com" target="_blank">qcolombet@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">Hi Sanjay,<div><br></div><div>I do not think this change is correct.</div><div>I have attached a test case with (before.ll) and without (after.ll) that change and the related C file (test.c).</div><div>To reproduce, compile with O1 or more and run the produced executable. The correct behavior is do not assert :).</div><div><br></div><div>Please revert or fix it.</div><div><br><div><span class=""><blockquote type="cite"><div>On Oct 10, 2014, at 4:00 PM, Sanjay Patel <<a href="mailto:spatel@rotateright.com" target="_blank">spatel@rotateright.com</a>> wrote:</div><br><div>Author: spatel<br>Date: Fri Oct 10 18:00:21 2014<br>New Revision: 219542<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=219542&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=219542&view=rev</a><br>Log:<br>Return undef on FP <-> Int conversions that overflow (PR21330).<br><br>The LLVM Lang Ref states for signed/unsigned int to float conversions:<br>"If the value cannot fit in the floating point value, the results are undefined."<br><br>And for FP to signed/unsigned int:<br>"If the value cannot fit in ty2, the results are undefined."<br><br>This matches the C definitions.<br></div></blockquote><div><br></div></span><div>Yes, this matches the C definitions, but the rules of propagating the undef value are not strictly the same in C and LLVM IR AFAICT.</div><div>Here is an example:</div><div>In (pseudo) C,</div><div>a = undef    // a is undef;</div><div>if (something)</div><div>  b = val;</div><div>else</div><div>  b = a;  // b is undef, but it should be the same value as a.</div><div>assert (something || b == a);</div><div><br></div><div>The related (pseudo) IR would be:</div><div>a = undef</div><div>b = select i1 something, val, a</div><div>cmpVal = cmp eq b, a</div><div>assertVal = or something, cmpVal</div><div>call assert(assertVal)</div><div><br></div><div>Now, the actual IR will be, because of constant propagation:</div><div><div>b = select i1 something, val, undef  // <— we lose the information that the next two undef are the same value.</div><div>cmpVal = cmp eq b, undef</div><div>assertVal = or something, cmpVal</div><div>call assert(assertVal)</div><div><br></div><div>This is still fine as long as undef are expanded into the same constant.</div><div>However, the LangRef states that the following transformation is valid:</div><div><pre style="overflow-x:auto;overflow-y:hidden;font-family:Consolas,'Deja Vu Sans Mono','Bitstream Vera Sans Mono',monospace;font-size:0.95em;line-height:15.960000038146973px;padding:0.5em;border:1px solid rgb(204,204,204);background-color:rgb(248,248,248)">  <span style="color:rgb(187,96,213)">%C</span> <span>=</span> <span style="color:rgb(0,112,32);font-weight:bold">select</span> <span style="color:rgb(187,96,213)">%X</span><span>,</span> <span style="color:rgb(187,96,213)">%Y</span><span>,</span> <span style="color:rgb(0,112,32);font-weight:bold">undef</span>
<span style="color:rgb(0,32,112);font-weight:bold">Safe:</span>
  <span style="color:rgb(187,96,213)">%C</span> <span>=</span> <span style="color:rgb(187,96,213)">%Y</span>
<span style="color:rgb(0,32,112);font-weight:bold">Unsafe:</span>
  <span style="color:rgb(187,96,213)">%C</span> <span>=</span> <span style="color:rgb(0,112,32);font-weight:bold">undef</span></pre><div>Therefore, we end up with the following IR:</div></div><div><div>b = val  // <— now, the value of b may not be a when ‘something' is false, which is incorrect. </div><div>cmpVal = cmp eq b, undef</div><div>assertVal = or something, cmpVal</div><div>call assert(assertVal)</div></div><div></div></div></div></div></div></blockquote></div><br>This entire thing is actually a matter of some debate. I'm trying to get all of it right below, but I've also CC-ed our resident language lawyer to help catch anywhere I get this wrong.</div><div class="gmail_extra"><br></div><div class="gmail_extra">I think how we interpret this hinges on one question first: do you consider "undefined results" to be equivalent to uninitialized memory? Both yes and no are reasonable, but they change how things proceed. Let's start with "yes".</div><div class="gmail_extra"><br></div><div class="gmail_extra">The C++ committee recently started changing the handling of an uninitialized variable to explicitly match the unpredictability of LLVM's undef because if there must be a single fixed value for all observations of the uninitialized variable, many implementation strategies for C++ become incredibly hard. Certain, LLVM currently could not implement anything else for uninitialized variables. I'm not 100% certain the C committee will follow suit, they may not. However, I'm frankly more concerned with what the C++ committee says about the semantics here than the C committee at this point... (color me biased, sorry.) So, if we want to consider this  "undefined result" to be the same as reading an uninitialized variable, I think the transformation is correct and the C test case is invalid. I think it becomes invalid when it uses an undefined value as the predicate to control flow. This is the classic "undefined value" transition to "undefined behavior" IMO.</div><div class="gmail_extra"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra">Ok, but what if we say "undefined results" are *not* equivalent to uninitialized memory, and instead being require to produce a specific unspecified value. I'm OK with this interpretation as well, but I think we should file a defect with the standard to get it clarified -- currently it isn't terribly clear for C++.</div><div class="gmail_extra"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra">I'm also pretty sure that much of this is actually *different* in C than in C++, and there are even defects filed against C in this area. =[</div><div class="gmail_extra"><br></div><div class="gmail_extra">Anyways, Richard can hopefully shed more direct light on this.</div></div>