SimplifyCFG or InstCombine incorrect in the presence of overflow

[alinas]

overflow_simplifycfg.ll.log

Running: opt -simplifycfg overflow_simplifycfg.ll shows the issue.
The branching is removed and the difference is done unconditionally. In the original test, the difference overflows.

This was found following the change in https://reviews.llvm.org/D98152. The test does not overflow following the SimplifyCFG transform but does overflow after Instcombine. I haven't understood why this is yet.

Pertinent diffs from original test:

Before SimplifyCFG:

40:                                               ; preds = %38, %13
  %41 = load i64, i64* %4, align 8
  %42 = getelementptr inbounds %"c442", %"c442"* %0, i64 0, i32 1
  %43 = load i64, i64* %42, align 8
  %44 = icmp slt i64 %41, %43
  br i1 %44, label %47, label %45

45:                                               ; preds = %40
  %46 = sub nsw i64 %41, %43
  br label %47

47:                                               ; preds = %45, %40
  %48 = phi i64 [ %46, %45 ], [ 0, %40 ]
  call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %9)
  ret i64 %48

After SimplifyCFG:

32:                                               ; preds = %29, %12
  %33 = load i64, i64* %4, align 8
  %34 = getelementptr inbounds %"c442", %"c442"* %0, i64 0, i32 1
  %35 = load i64, i64* %34, align 8
  %36 = icmp slt i64 %33, %35
  %37 = sub nsw i64 %33, %35
  %spec.select = select i1 %36, i64 0, i64 %37
  call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %7)
  ret i64 %spec.select

Before instcombine:

%112 = icmp slt i64 %.0, %111
%113 = sub nsw i64 %.0, %111
%spec.select = select i1 %112, i64 0, i64 %113
ret i64 %spec.select

After instcombine:

%112 = sub nsw i64 %.0, %111
%113 = call i64 @llvm.smax.i64(i64 %112, i64 0)
ret i64 %113

[alinas]

@nikic @spatel-gh

[LebedevRI]

Hm, yes: https://alive2.llvm.org/ce/z/b2fxWg

[LebedevRI]

Ok, the problem is in instcombine: https://alive2.llvm.org/ce/z/y9Dp8A

[nikic]

Probably these SPF patterns need to be removed:

llvm-project/llvm/lib/Analysis/ValueTracking.cpp

Lines 5805 to 5817 in abc8ca6

	// Z = X -nsw Y
	// (X >s Y) ? 0 : Z ==> (Z >s 0) ? 0 : Z ==> SMIN(Z, 0)
	// (X <s Y) ? 0 : Z ==> (Z <s 0) ? 0 : Z ==> SMAX(Z, 0)
	if (match(TrueVal, m_Zero()) &&
	match(FalseVal, m_NSWSub(m_Specific(CmpLHS), m_Specific(CmpRHS))))
	return {Pred == CmpInst::ICMP_SGT ? SPF_SMIN : SPF_SMAX, SPNB_NA, false};
	// Z = X -nsw Y
	// (X >s Y) ? Z : 0 ==> (Z >s 0) ? Z : 0 ==> SMAX(Z, 0)
	// (X <s Y) ? Z : 0 ==> (Z <s 0) ? Z : 0 ==> SMIN(Z, 0)
	if (match(FalseVal, m_Zero()) &&
	match(TrueVal, m_NSWSub(m_Specific(CmpLHS), m_Specific(CmpRHS))))
	return {Pred == CmpInst::ICMP_SGT ? SPF_SMAX : SPF_SMIN, SPNB_NA, false};

[nikic]

These were added in cfcc42b.

[rotateright]

Thanks for the report and analysis! I'll make a patch to remove the wrong analysis.