[PATCH] Fixing a corner-case bug in lib call optimization

Eli Friedman eli.friedman at gmail.com
Wed Aug 14 10:52:19 PDT 2013


On Tue, Aug 13, 2013 at 10:36 PM, Nick Lewycky <nicholas at mxc.ca> wrote:

> Gao, Yunzhong wrote:
>
>> Hi Nick,
>> Thanks for reviewing the patch.
>> I will change the type conversion to "unsigned char" instead; that should
>> make
>> the result deterministic.
>> I feel that this is probably still not an ideal solution because the host
>> machine
>> might have a different size for "char" type than the target machine, but
>> in the
>> llvm backend I did not find a way to extract the size of target "char"
>> type. I
>> would guess that machines with non-8-bit bytes are rare nowadays? Since
>> the patch is only intended for a corner-case comparing a value with zero,
>> it
>> might suffice using a cast to "unsigned char" or even "(value&  255)".
>> What do you think?
>>
>
> Actually, I was thinking about this some more and isn't the testcase you
> posted undefined behaviour?
>
> The C standard specifies that it gets converted from int to char. That
> conversion will produce an implementation-defined result when the int is
> not in range of a char. Now for the crazy part -- isn't it true that there
> is no rule requiring a value in the abstract machine of C (or C++) to
> actually fit in the number of bits allotted? Given the right setup, you can
> have char x == 1000 evaluate to true (even assuming 8-bit char), and no
> undefined behaviour, merely implementation-defined results.
>
> Maybe I'm crazy?
>

It's implementation-defined, not undefined... and every platform LLVM
supports (or is ever likely to support) defines the behavior to be exactly
what this patch implements.

And in fact, there isn't any way to have char x == 1000 to evaluate to true
if char is 8 bits: the combination of 6.2.6.2p1 and 6.2.5p9 prohibits it.

-Eli
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130814/d9c38d3f/attachment.html>


More information about the llvm-commits mailing list