Skip to content

Commit 4d2ae88

Browse files
committed
[InstCombine] Fix invalid scalarization of div
If the binop is not speculatable, and the extract index is out of range, then scalarizing will perform the operation on a poison operand, resulting in immediate UB, instead of the previous poison result. Fixes #97053.
1 parent a357137 commit 4d2ae88

File tree

2 files changed

+8
-4
lines changed

2 files changed

+8
-4
lines changed

llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,13 +419,15 @@ Instruction *InstCombinerImpl::visitExtractElementInst(ExtractElementInst &EI) {
419419
// If extracting a specified index from the vector, see if we can recursively
420420
// find a previously computed scalar that was inserted into the vector.
421421
auto *IndexC = dyn_cast<ConstantInt>(Index);
422+
bool HasKnownValidIndex = false;
422423
if (IndexC) {
423424
// Canonicalize type of constant indices to i64 to simplify CSE
424425
if (auto *NewIdx = getPreferredVectorIndex(IndexC))
425426
return replaceOperand(EI, 1, NewIdx);
426427

427428
ElementCount EC = EI.getVectorOperandType()->getElementCount();
428429
unsigned NumElts = EC.getKnownMinValue();
430+
HasKnownValidIndex = IndexC->getValue().ult(NumElts);
429431

430432
if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(SrcVec)) {
431433
Intrinsic::ID IID = II->getIntrinsicID();
@@ -471,8 +473,11 @@ Instruction *InstCombinerImpl::visitExtractElementInst(ExtractElementInst &EI) {
471473
return UnaryOperator::CreateWithCopiedFlags(UO->getOpcode(), E, UO);
472474
}
473475

476+
// If the binop is not speculatable, we cannot hoist the extractelement if
477+
// it may make the operand poison.
474478
BinaryOperator *BO;
475-
if (match(SrcVec, m_BinOp(BO)) && cheapToScalarize(SrcVec, Index)) {
479+
if (match(SrcVec, m_BinOp(BO)) && cheapToScalarize(SrcVec, Index) &&
480+
(HasKnownValidIndex || isSafeToSpeculativelyExecute(BO))) {
476481
// extelt (binop X, Y), Index --> binop (extelt X, Index), (extelt Y, Index)
477482
Value *X = BO->getOperand(0), *Y = BO->getOperand(1);
478483
Value *E0 = Builder.CreateExtractElement(X, Index);

llvm/test/Transforms/InstCombine/scalarization.ll

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,12 +158,11 @@ define i8 @extract_element_binop_splat_variable_index(<4 x i8> %x, i32 %y) {
158158

159159
; We cannot move the extractelement before the sdiv here, because %z may be
160160
; out of range, making the divisor poison and resulting in immediate UB.
161-
; FIXME: This is a miscompile.
162161
define i8 @extract_element_binop_splat_variable_index_may_trap(<4 x i8> %x, <4 x i8> %y, i32 %z) {
163162
;
164163
; CHECK-LABEL: @extract_element_binop_splat_variable_index_may_trap(
165-
; CHECK-NEXT: [[TMP1:%.*]] = extractelement <4 x i8> [[Y:%.*]], i32 [[Z:%.*]]
166-
; CHECK-NEXT: [[R:%.*]] = sdiv i8 42, [[TMP1]]
164+
; CHECK-NEXT: [[B:%.*]] = sdiv <4 x i8> <i8 42, i8 42, i8 42, i8 42>, [[Y:%.*]]
165+
; CHECK-NEXT: [[R:%.*]] = extractelement <4 x i8> [[B]], i32 [[Z:%.*]]
167166
; CHECK-NEXT: ret i8 [[R]]
168167
;
169168
%b = sdiv <4 x i8> splat (i8 42), %y

0 commit comments

Comments
 (0)