llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.27k stars 11.67k forks source link

Wrong code at -O1/O2 on x86_64-linux_gnu since Clang-16 #62992

Closed shao-hua-li closed 1 year ago

shao-hua-li commented 1 year ago

This seems to be a quite recent regression. opt-bisect-limit suggests that the bug might be in IndVarSimplifyPass.

Compiler explorer: https://godbolt.org/z/zobd581cf

% cat a.c
int a, b, d;
unsigned c;
int main() {
  int e = 4;
  for (; e; e--) {
    b = 4;
    for (; ((b + 61) << (b + 4)) - 16636 >= 0; b--) {
      c++;
      d = a > 0 && c < 7 / a || 0;
    }
  }
  return 0;
}
%
% clang-tk -O1 a.c && ./a.out
Floating point exception
% clang-tk -O2 a.c && ./a.out
Floating point exception
% clang-tk -fsanitize=address,undefined a.c && ./a.out
%

I have reported another with a similar test case, not sure if they have the same root cause: https://github.com/llvm/llvm-project/issues/62515

nikic commented 1 year ago

Slightly reduced IR before IndVarSimplify:

@b = dso_local local_unnamed_addr global i32 0, align 4
@c = dso_local local_unnamed_addr global i32 0, align 4
@a = dso_local local_unnamed_addr global i32 0, align 4
@d = dso_local local_unnamed_addr global i32 0, align 4

define i32 @main() {
entry:
  %0 = load i32, ptr @a, align 4
  %cmp4 = icmp sgt i32 %0, 0
  %c.promoted13 = load i32, ptr @c, align 4
  br label %for.cond1.preheader

for.cond1.preheader:                              ; preds = %entry, %for.inc6
  %e.018 = phi i32 [ 4, %entry ], [ %dec7, %for.inc6 ]
  %inc.lcssa1417 = phi i32 [ %c.promoted13, %entry ], [ %inc.lcssa, %for.inc6 ]
  br label %for.body3

for.body3:                                        ; preds = %for.cond1.preheader, %lor.end
  %storemerge12 = phi i32 [ 4, %for.cond1.preheader ], [ %dec, %lor.end ]
  %inc1011 = phi i32 [ %inc.lcssa1417, %for.cond1.preheader ], [ %inc, %lor.end ]
  %inc = add i32 %inc1011, 1
  br i1 %cmp4, label %land.lhs.true, label %lor.rhs

land.lhs.true:                                    ; preds = %for.body3
  %div = udiv i32 7, %0
  %cmp5 = icmp ult i32 %inc, %div
  br i1 %cmp5, label %lor.end, label %lor.rhs

lor.rhs:                                          ; preds = %land.lhs.true, %for.body3
  br label %lor.end

lor.end:                                          ; preds = %lor.rhs, %land.lhs.true
  %lor.ext = phi i32 [ 1, %land.lhs.true ], [ 0, %lor.rhs ]
  %dec = add nsw i32 %storemerge12, -1
  %add = add nsw i32 %storemerge12, 60
  %add2 = add nsw i32 %storemerge12, 3
  %shl = shl i32 %add, %add2
  %cmp = icmp sgt i32 %shl, 16635
  br i1 %cmp, label %for.body3, label %for.inc6

for.inc6:                                         ; preds = %lor.end
  %lor.ext.lcssa = phi i32 [ %lor.ext, %lor.end ]
  %dec.lcssa = phi i32 [ %dec, %lor.end ]
  %inc.lcssa = phi i32 [ %inc, %lor.end ]
  %dec7 = add nsw i32 %e.018, -1
  %tobool.not = icmp eq i32 %dec7, 0
  br i1 %tobool.not, label %for.end8, label %for.cond1.preheader

for.end8:                                         ; preds = %for.inc6
  %lor.ext.lcssa.lcssa = phi i32 [ %lor.ext.lcssa, %for.inc6 ]
  %dec.lcssa.lcssa = phi i32 [ %dec.lcssa, %for.inc6 ]
  %inc.lcssa.lcssa = phi i32 [ %inc.lcssa, %for.inc6 ]
  store i32 %inc.lcssa.lcssa, ptr @c, align 4
  store i32 %lor.ext.lcssa.lcssa, ptr @d, align 4
  store i32 %dec.lcssa.lcssa, ptr @b, align 4
  ret i32 0
}

We do appear to materialize a udiv in the preheader somewhere in simplifyAndExtend().

nikic commented 1 year ago

I believe the problem is that makeIVComparisonInvariant() fails to ensure that that expansion is safe.

nikic commented 1 year ago

Reduced test case:

declare void @use(i1)

define i32 @test(i32 %arg) {
entry:
  br label %loop

loop:
  %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop.latch ]
  %iv.next = add i32 %iv, 1
  %cmp = icmp eq i32 %iv, 1
  br i1 %cmp, label %if, label %loop.latch

if:
  %div = udiv i32 7, %arg
  %cmp2 = icmp ult i32 %iv.next, %div
  call void @use(i1 %cmp2)
  br label %loop.latch

loop.latch:
  br i1 false, label %loop, label %exit

exit:
  %inc.lcssa = phi i32 [ %iv.next, %loop.latch ]
  ret i32 %inc.lcssa
}