[llvm] r219542 - Return undef on FP <-> Int conversions that overflow (PR21330).

[Chandler Carruth]

On Fri, Oct 17, 2014 at 5:17 PM, Quentin Colombet <qcolombet at apple.com>
wrote:

> Hi Sanjay,
>
> I do not think this change is correct.
> I have attached a test case with (before.ll) and without (after.ll) that
> change and the related C file (test.c).
> To reproduce, compile with O1 or more and run the produced executable. The
> correct behavior is do not assert :).
>
> Please revert or fix it.
>
> On Oct 10, 2014, at 4:00 PM, Sanjay Patel <spatel at rotateright.com> wrote:
>
> Author: spatel
> Date: Fri Oct 10 18:00:21 2014
> New Revision: 219542
>
> URL: http://llvm.org/viewvc/llvm-project?rev=219542&view=rev
> Log:
> Return undef on FP <-> Int conversions that overflow (PR21330).
>
> The LLVM Lang Ref states for signed/unsigned int to float conversions:
> "If the value cannot fit in the floating point value, the results are
> undefined."
>
> And for FP to signed/unsigned int:
> "If the value cannot fit in ty2, the results are undefined."
>
> This matches the C definitions.
>
>
> 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.
> Here is an example:
> In (pseudo) C,
> a = undef    // a is undef;
> if (something)
>   b = val;
> else
>   b = a;  // b is undef, but it should be the same value as a.
> assert (something || b == a);
>
> The related (pseudo) IR would be:
> a = undef
> b = select i1 something, val, a
> cmpVal = cmp eq b, a
> assertVal = or something, cmpVal
> call assert(assertVal)
>
> Now, the actual IR will be, because of constant propagation:
> b = select i1 something, val, undef  // <— we lose the information that
> the next two undef are the same value.
> cmpVal = cmp eq b, undef
> assertVal = or something, cmpVal
> call assert(assertVal)
>
> This is still fine as long as undef are expanded into the same constant.
> However, the LangRef states that the following transformation is valid:
>
>   %C = select %X, %Y, undefSafe:
>   %C = %YUnsafe:
>   %C = undef
>
> Therefore, we end up with the following IR:
> b = val  // <— now, the value of b may not be a when ‘something' is false,
> which is incorrect.
> cmpVal = cmp eq b, undef
> assertVal = or something, cmpVal
> call assert(assertVal)
>

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.

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".

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.


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++.


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. =[

Anyways, Richard can hopefully shed more direct light on this.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141017/ff614dc0/attachment.html>