[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added subscribers: eugenis, guiand, vitalybuka.
vitalybuka added a comment.

After this patch we have false msan reports on code like this:

  bool iv_compare2(const int *op1, const int *op2) {
    if (op1[1] != op2[1]) return op1[1] < op2[1];
    for (int i = 1; i >= 0; i--) {
      if (op1[i] != op2[i]) return op1[i] < op2[i];
    }
    return false;
  }
  
  void foo() {
    int a[2] = {};
    int b[2] = {};
    auto UNINITIALIZED= iv_compare2(a, b);
  }

Here it looks fine and the same as before the patch. It returns "and undef, false" which should be false.

  *** IR Dump After Simplify the CFG ***
  ; Function Attrs: norecurse nounwind readonly sanitize_memory uwtable
  define zeroext i1 @_Z11iv_compare2PKiS0_(i32* nocapture readonly %op1, i32* nocapture readonly %op2) local_unnamed_addr #0 !dbg !8 {
  entry:
    %arrayidx = getelementptr inbounds i32, i32* %op1, i64 1, !dbg !10
    %0 = load i32, i32* %arrayidx, align 4, !dbg !10
    %arrayidx1 = getelementptr inbounds i32, i32* %op2, i64 1, !dbg !11
    %1 = load i32, i32* %arrayidx1, align 4, !dbg !11
    %cmp.not = icmp eq i32 %0, %1, !dbg !12
    br i1 %cmp.not, label %for.cond, label %if.then, !dbg !10
  
  if.then:                                          ; preds = %entry
    %cmp4 = icmp slt i32 %0, %1, !dbg !13
    ret i1 %cmp4, !dbg !14
  
  for.cond:                                         ; preds = %entry
    %2 = load i32, i32* %op1, align 4, !dbg !15
    %3 = load i32, i32* %op2, align 4, !dbg !16
    %cmp9.not.1 = icmp eq i32 %2, %3, !dbg !17
    %cmp15 = icmp slt i32 %2, %3
    %spec.select39 = select i1 %cmp9.not.1, i1 undef, i1 %cmp15, !dbg !15
    %spec.select40 = select i1 %cmp9.not.1, i1 false, i1 true, !dbg !15
    %spec.select = and i1 %spec.select39, %spec.select40
    ret i1 %spec.select
  }

However with this patch after the next transformation it breaks the code:
Now it returns undef instead of false if %2 == %3

  *** IR Dump After Combine redundant instructions ***
  ; Function Attrs: norecurse nounwind readonly sanitize_memory uwtable
  define zeroext i1 @_Z11iv_compare2PKiS0_(i32* nocapture readonly %op1, i32* nocapture readonly %op2) local_unnamed_addr #0 !dbg !8 {
  entry:
    %arrayidx = getelementptr inbounds i32, i32* %op1, i64 1, !dbg !10
    %0 = load i32, i32* %arrayidx, align 4, !dbg !10
    %arrayidx1 = getelementptr inbounds i32, i32* %op2, i64 1, !dbg !11
    %1 = load i32, i32* %arrayidx1, align 4, !dbg !11
    %cmp.not = icmp eq i32 %0, %1, !dbg !12
    br i1 %cmp.not, label %for.cond, label %if.then, !dbg !10
  
  if.then:                                          ; preds = %entry
    %cmp4 = icmp slt i32 %0, %1, !dbg !13
    ret i1 %cmp4, !dbg !14
  
  for.cond:                                         ; preds = %entry
    %2 = load i32, i32* %op1, align 4, !dbg !15
    %3 = load i32, i32* %op2, align 4, !dbg !16
    %cmp9.not.1 = icmp eq i32 %2, %3, !dbg !17
    %cmp15 = icmp slt i32 %2, %3
    %spec.select39 = select i1 %cmp9.not.1, i1 undef, i1 %cmp15, !dbg !15
    ret i1 %spec.select39
  }

The msan reasonably reports a bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83360