Author: Roman Lebedev Date: 2020-12-18T21:18:13+03:00 New Revision: e9289dc25f7923abff9ac762bd3f6819c0605f20
URL: https://github.com/llvm/llvm-project/commit/e9289dc25f7923abff9ac762bd3f6819c0605f20 DIFF: https://github.com/llvm/llvm-project/commit/e9289dc25f7923abff9ac762bd3f6819c0605f20.diff LOG: [InstSimplify] Don't miscompile `X == 0 ? abs(X) : -abs(X) --> -abs(X)` xform The transform wasn't checking that the LHS of the comparison *is* the `X` in question... This is the miscompile that was holding up D87188. Thanks to Dave Green for producing an actionable reproducer! Added: Modified: llvm/lib/Analysis/InstructionSimplify.cpp llvm/test/Transforms/InstSimplify/abs_intrinsic.ll Removed: ################################################################################ diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp index 35c21a023bae..55f3bc4f2923 100644 --- a/llvm/lib/Analysis/InstructionSimplify.cpp +++ b/llvm/lib/Analysis/InstructionSimplify.cpp @@ -4020,11 +4020,12 @@ static Value *simplifySelectWithICmpCond(Value *CondVal, Value *TrueVal, // X == 0 ? abs(X) : -abs(X) --> -abs(X) // X == 0 ? -abs(X) : abs(X) --> abs(X) - if (match(TrueVal, m_Intrinsic<Intrinsic::abs>(m_Value(X))) && - match(FalseVal, m_Neg(m_Intrinsic<Intrinsic::abs>(m_Specific(X))))) + if (match(TrueVal, m_Intrinsic<Intrinsic::abs>(m_Specific(CmpLHS))) && + match(FalseVal, m_Neg(m_Intrinsic<Intrinsic::abs>(m_Specific(CmpLHS))))) return FalseVal; - if (match(TrueVal, m_Neg(m_Intrinsic<Intrinsic::abs>(m_Value(X)))) && - match(FalseVal, m_Intrinsic<Intrinsic::abs>(m_Specific(X)))) + if (match(TrueVal, + m_Neg(m_Intrinsic<Intrinsic::abs>(m_Specific(CmpLHS)))) && + match(FalseVal, m_Intrinsic<Intrinsic::abs>(m_Specific(CmpLHS)))) return FalseVal; } diff --git a/llvm/test/Transforms/InstSimplify/abs_intrinsic.ll b/llvm/test/Transforms/InstSimplify/abs_intrinsic.ll index e0bb1704bc48..9d5980fbf782 100644 --- a/llvm/test/Transforms/InstSimplify/abs_intrinsic.ll +++ b/llvm/test/Transforms/InstSimplify/abs_intrinsic.ll @@ -225,7 +225,10 @@ define i32 @select_abs_of_abs_eq(i32 %x) { define i32 @select_abs_of_abs_eq_wrong(i32 %x, i32 %y) { ; CHECK-LABEL: @select_abs_of_abs_eq_wrong( ; CHECK-NEXT: [[ABS:%.*]] = call i32 @llvm.abs.i32(i32 [[X:%.*]], i1 false) -; CHECK-NEXT: ret i32 [[ABS]] +; CHECK-NEXT: [[NEG:%.*]] = sub i32 0, [[ABS]] +; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[Y:%.*]], 0 +; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 [[NEG]], i32 [[ABS]] +; CHECK-NEXT: ret i32 [[SEL]] ; %abs = call i32 @llvm.abs.i32(i32 %x, i1 false) %neg = sub i32 0, %abs @@ -264,7 +267,9 @@ define i32 @select_nabs_of_abs_eq_wrong(i32 %x, i32 %y) { ; CHECK-LABEL: @select_nabs_of_abs_eq_wrong( ; CHECK-NEXT: [[ABS:%.*]] = call i32 @llvm.abs.i32(i32 [[X:%.*]], i1 false) ; CHECK-NEXT: [[NEG:%.*]] = sub i32 0, [[ABS]] -; CHECK-NEXT: ret i32 [[NEG]] +; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[Y:%.*]], 0 +; CHECK-NEXT: [[SEL:%.*]] = select i1 [[CMP]], i32 [[ABS]], i32 [[NEG]] +; CHECK-NEXT: ret i32 [[SEL]] ; %abs = call i32 @llvm.abs.i32(i32 %x, i1 false) %neg = sub i32 0, %abs _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
