diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp index d4b4a6de1256f..e5916981fc3e5 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp @@ -429,6 +429,21 @@ Instruction *InstCombinerImpl::foldSelectOpOp(SelectInst &SI, Instruction *TI, !OtherOpF->getType()->isVectorTy())) return nullptr; + // If we are sinking div/rem after a select, we may need to freeze the + // condition because div/rem may induce immediate UB with a poison operand. + // For example, the following transform is not safe if Cond can ever be poison + // because we can replace poison with zero and then we have div-by-zero that + // didn't exist in the original code: + // Cond ? x/y : x/z --> x / (Cond ? y : z) + auto *BO = dyn_cast(TI); + if (BO && BO->isIntDivRem() && !isGuaranteedNotToBePoison(Cond)) { + // A udiv/urem with a common divisor is safe because UB can only occur with + // div-by-zero, and that would be present in the original code. + if (BO->getOpcode() == Instruction::SDiv || + BO->getOpcode() == Instruction::SRem || MatchIsOpZero) + Cond = Builder.CreateFreeze(Cond); + } + // If we reach here, they do have operations in common. Value *NewSI = Builder.CreateSelect(Cond, OtherOpT, OtherOpF, SI.getName() + ".v", &SI); diff --git a/llvm/test/Transforms/InstCombine/select-divrem.ll b/llvm/test/Transforms/InstCombine/select-divrem.ll index 0c81161c9cbc6..90e9919f31dac 100644 --- a/llvm/test/Transforms/InstCombine/select-divrem.ll +++ b/llvm/test/Transforms/InstCombine/select-divrem.ll @@ -1,9 +1,14 @@ ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py ; RUN: opt < %s -passes=instcombine -S | FileCheck %s +; It is a miscompile in most of these tests if we +; execute div/rem without freezing the potentially +; poison condition value. + define i5 @sdiv_common_divisor(i1 %b, i5 %x, i5 %y, i5 %z) { ; CHECK-LABEL: @sdiv_common_divisor( -; CHECK-NEXT: [[SEL_V:%.*]] = select i1 [[B:%.*]], i5 [[Z:%.*]], i5 [[Y:%.*]] +; CHECK-NEXT: [[TMP1:%.*]] = freeze i1 [[B:%.*]] +; CHECK-NEXT: [[SEL_V:%.*]] = select i1 [[TMP1]], i5 [[Z:%.*]], i5 [[Y:%.*]] ; CHECK-NEXT: [[SEL:%.*]] = sdiv i5 [[SEL_V]], [[X:%.*]] ; CHECK-NEXT: ret i5 [[SEL]] ; @@ -15,7 +20,8 @@ define i5 @sdiv_common_divisor(i1 %b, i5 %x, i5 %y, i5 %z) { define i5 @srem_common_divisor(i1 %b, i5 %x, i5 %y, i5 %z) { ; CHECK-LABEL: @srem_common_divisor( -; CHECK-NEXT: [[SEL_V:%.*]] = select i1 [[B:%.*]], i5 [[Z:%.*]], i5 [[Y:%.*]] +; CHECK-NEXT: [[TMP1:%.*]] = freeze i1 [[B:%.*]] +; CHECK-NEXT: [[SEL_V:%.*]] = select i1 [[TMP1]], i5 [[Z:%.*]], i5 [[Y:%.*]] ; CHECK-NEXT: [[SEL:%.*]] = srem i5 [[SEL_V]], [[X:%.*]] ; CHECK-NEXT: ret i5 [[SEL]] ; @@ -25,6 +31,9 @@ define i5 @srem_common_divisor(i1 %b, i5 %x, i5 %y, i5 %z) { ret i5 %sel } +; This is ok without freeze because UB can only happen with x==0, +; and that occurs in the original code. + define i5 @udiv_common_divisor(i1 %b, i5 %x, i5 %y, i5 %z) { ; CHECK-LABEL: @udiv_common_divisor( ; CHECK-NEXT: [[SEL_V:%.*]] = select i1 [[B:%.*]], i5 [[Z:%.*]], i5 [[Y:%.*]] @@ -37,6 +46,9 @@ define i5 @udiv_common_divisor(i1 %b, i5 %x, i5 %y, i5 %z) { ret i5 %sel } +; This is ok without freeze because UB can only happen with x==0, +; and that occurs in the original code. + define i5 @urem_common_divisor(i1 %b, i5 %x, i5 %y, i5 %z) { ; CHECK-LABEL: @urem_common_divisor( ; CHECK-NEXT: [[SEL_V:%.*]] = select i1 [[B:%.*]], i5 [[Z:%.*]], i5 [[Y:%.*]] @@ -51,7 +63,8 @@ define i5 @urem_common_divisor(i1 %b, i5 %x, i5 %y, i5 %z) { define i5 @sdiv_common_dividend(i1 %b, i5 %x, i5 %y, i5 %z) { ; CHECK-LABEL: @sdiv_common_dividend( -; CHECK-NEXT: [[SEL_V:%.*]] = select i1 [[B:%.*]], i5 [[Z:%.*]], i5 [[Y:%.*]] +; CHECK-NEXT: [[TMP1:%.*]] = freeze i1 [[B:%.*]] +; CHECK-NEXT: [[SEL_V:%.*]] = select i1 [[TMP1]], i5 [[Z:%.*]], i5 [[Y:%.*]] ; CHECK-NEXT: [[SEL:%.*]] = sdiv i5 [[X:%.*]], [[SEL_V]] ; CHECK-NEXT: ret i5 [[SEL]] ; @@ -63,7 +76,8 @@ define i5 @sdiv_common_dividend(i1 %b, i5 %x, i5 %y, i5 %z) { define i5 @srem_common_dividend(i1 %b, i5 %x, i5 %y, i5 %z) { ; CHECK-LABEL: @srem_common_dividend( -; CHECK-NEXT: [[SEL_V:%.*]] = select i1 [[B:%.*]], i5 [[Z:%.*]], i5 [[Y:%.*]] +; CHECK-NEXT: [[TMP1:%.*]] = freeze i1 [[B:%.*]] +; CHECK-NEXT: [[SEL_V:%.*]] = select i1 [[TMP1]], i5 [[Z:%.*]], i5 [[Y:%.*]] ; CHECK-NEXT: [[SEL:%.*]] = srem i5 [[X:%.*]], [[SEL_V]] ; CHECK-NEXT: ret i5 [[SEL]] ; @@ -75,7 +89,8 @@ define i5 @srem_common_dividend(i1 %b, i5 %x, i5 %y, i5 %z) { define i5 @udiv_common_dividend(i1 %b, i5 %x, i5 %y, i5 %z) { ; CHECK-LABEL: @udiv_common_dividend( -; CHECK-NEXT: [[SEL_V:%.*]] = select i1 [[B:%.*]], i5 [[Z:%.*]], i5 [[Y:%.*]] +; CHECK-NEXT: [[TMP1:%.*]] = freeze i1 [[B:%.*]] +; CHECK-NEXT: [[SEL_V:%.*]] = select i1 [[TMP1]], i5 [[Z:%.*]], i5 [[Y:%.*]] ; CHECK-NEXT: [[SEL:%.*]] = udiv i5 [[X:%.*]], [[SEL_V]] ; CHECK-NEXT: ret i5 [[SEL]] ; @@ -87,7 +102,8 @@ define i5 @udiv_common_dividend(i1 %b, i5 %x, i5 %y, i5 %z) { define i5 @urem_common_dividend(i1 %b, i5 %x, i5 %y, i5 %z) { ; CHECK-LABEL: @urem_common_dividend( -; CHECK-NEXT: [[SEL_V:%.*]] = select i1 [[B:%.*]], i5 [[Z:%.*]], i5 [[Y:%.*]] +; CHECK-NEXT: [[TMP1:%.*]] = freeze i1 [[B:%.*]] +; CHECK-NEXT: [[SEL_V:%.*]] = select i1 [[TMP1]], i5 [[Z:%.*]], i5 [[Y:%.*]] ; CHECK-NEXT: [[SEL:%.*]] = urem i5 [[X:%.*]], [[SEL_V]] ; CHECK-NEXT: ret i5 [[SEL]] ; @@ -97,6 +113,11 @@ define i5 @urem_common_dividend(i1 %b, i5 %x, i5 %y, i5 %z) { ret i5 %sel } +; Repeat the above tests, but guarantee that the select +; condition is not poison via argument attribute. That +; makes it safe to execute the select before div/rem +; without needing to freeze the condition. + define i5 @sdiv_common_divisor_defined_cond(i1 noundef %b, i5 %x, i5 %y, i5 %z) { ; CHECK-LABEL: @sdiv_common_divisor_defined_cond( ; CHECK-NEXT: [[SEL_V:%.*]] = select i1 [[B:%.*]], i5 [[Z:%.*]], i5 [[Y:%.*]]