[PATCH] D92270: [ConstantFold] Fold more operations to poison

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

Hi,

I'm seeing a miscompile with this patch. I'm not very good at the semantic differences between undef and poison so I don't know really where it goes wrong.

Before Early CSE I see this in the input:

  entry:
    %cmp1 = icmp uge i16 1015, 16, !dbg !9
    %shr = lshr i16 -1, 1015
    %cmp2 = icmp ugt i16 2, %shr
    %or.cond = or i1 %cmp1, %cmp2, !dbg !10
    br i1 %or.cond, label %if.then3, label %if.else4, !dbg !10

This corresponds to the following C-code:

  int16_t y1 = 1022;
  uint16_t res1 = 0;
  if (y1 < 0)
    res1 = 0;
  else
    res1 = 1015;
  
  if ((res1 >= 16) || (2 > (65535u >> res1)))
    res2 = 42;
  else
    res2 = 1;

So in the C input, the right shift is guarded and won't be carried out if res1 is too large to avoid UB, but in the ll file the lshr is executed unconditionally.

Then after Early CSE I instead get

  entry:
    br i1 poison, label %if.then3, label %if.else4, !dbg !9

So I suppose, with this patch the lshr will result in poison, and that poison will then make both %cmp2 and %or.cond be poison. And then we don't know where to jump so res2 gets the wrong value.

I'm not very good at poison semantics, but it seems to me that either the input to ewarly cse is invalid, or the patch has broken the semantics of "or"?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92270/new/

https://reviews.llvm.org/D92270