[llvm-dev] RFC: Killing undef and spreading poison

[Friedman, Eli via llvm-dev]

On 10/18/2016 12:25 PM, Nuno Lopes wrote:
>> On 10/18/2016 5:06 AM, Nuno Lopes via llvm-dev wrote:
>>> Another use is, for example, to implement bit-fields:
>>> a.x = 2
>>> becomes:
>>> %v = load %a
>>> %v2 = freeze %v  ; %v could be uninitialized data (poison)
>>> %v3 = ... bitmasking...
>>> store %a, %v3
>>
>> It seems like you're saying that an integer load which touches any 
>> uninitialized byte of memory results in poison.  Therefore, load %a 
>> simplifies to "poison", and your proposed lowering of a bitfield 
>> write throws away the value of every other adjacent bitfield.  Am I 
>> missing something?
>
> Right, a load touching a single uninitialized bit results in poison.
> The trick is that on the first bitfield write, all the remaining 
> untouched fields become initialized (with an arbitrary value, though). 
> Essentially we are making the adjacent bitfields undef.
>
> So if you have:
> struct foo a;
> a.x = 2;
> a.y = 3;
>
> IR becomes:
> %a = alloca foo
> %x = load %a
> %x2 = freeze %v  ; %x2 not poison
> %x3 = bitmasking  ; %x3 not poison
> store %a, %x3
>
> %y = load %a
> %y2 = freeze %y  ;  not needed; %y is not poison
> etc..
>
> Nuno
Oh, okay, that works.  I should have thought that through a bit more 
carefully.

Sort of related testcase:

struct S { int x : 24; };
S s = { 2 };

Gives:

@s = local_unnamed_addr global { i8, i8, i8, i8 } { i8 2, i8 0, i8 0, i8 
undef }, align 4

When undef goes away, clang will need to be patched to generate zero 
here?  More generally, will struct padding in globals be poison, or some 
arbitrary value?


A couple of related topics:

Instcombine currently transforms "memcpy(dst, src, 4)" to "load i32 src; 
store i32 dst".  I guess that's illegal under this model?  How about the 
related transform "memcpy(dst, src, 4)" to "load <4 x i8> src; store <4 
x i8> dst"?  (SROA also performs similar transforms.)

Have you given any thought to how auto-upgrade for bitcode files would 
work?  I guess in general, you can upgrade an old "load i32" to "bitcast 
(freeze (load <4 x i8>)) to i32", but that's really awkward.

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project