Skip to content

Commit

Permalink
[InstCombine] replace undef in vector constant for safe shift transfo…
Browse files Browse the repository at this point in the history
…rm (PR45447)

As noted in PR45447, we have a vector-constant-with-undef-element transform bug:
https://bugs.llvm.org/show_bug.cgi?id=45447

We replace undefs with a safe constant (0 or -1) based on the (non-)negative
predicate constraint.

So this is correct:
http://volta.cs.utah.edu:8080/z/WZE36H
...but this is not:
http://volta.cs.utah.edu:8080/z/boj8gJ

Previously, we were relying on getSafeVectorConstantForBinop() in the related fold (D76800).
But that's making an assumption about what qualifies as "safe", and that assumption may
not always hold.

Differential Revision: https://reviews.llvm.org/D77739
  • Loading branch information
rotateright committed Apr 9, 2020
1 parent 8b3d392 commit 812970e
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 6 deletions.
16 changes: 11 additions & 5 deletions llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
Expand Up @@ -3071,16 +3071,22 @@ Instruction *InstCombiner::visitXor(BinaryOperator &I) {
Constant *C;
if (match(NotVal, m_AShr(m_Constant(C), m_Value(Y))) &&
match(C, m_Negative())) {
Constant *NewC = ConstantExpr::getNot(C);
if (C->getType()->isVectorTy())
NewC = getSafeVectorConstantForBinop(Instruction::LShr, NewC, false);
return BinaryOperator::CreateLShr(NewC, Y);
// We matched a negative constant, so propagating undef is unsafe.
// Clamp undef elements to -1.
Type *EltTy = C->getType()->getScalarType();
C = Constant::replaceUndefsWith(C, ConstantInt::getAllOnesValue(EltTy));
return BinaryOperator::CreateLShr(ConstantExpr::getNot(C), Y);
}

// ~(C >>u Y) --> ~C >>s Y (when inverting the replicated sign bits)
if (match(NotVal, m_LShr(m_Constant(C), m_Value(Y))) &&
match(C, m_NonNegative()))
match(C, m_NonNegative())) {
// We matched a non-negative constant, so propagating undef is unsafe.
// Clamp undef elements to 0.
Type *EltTy = C->getType()->getScalarType();
C = Constant::replaceUndefsWith(C, ConstantInt::getNullValue(EltTy));
return BinaryOperator::CreateAShr(ConstantExpr::getNot(C), Y);
}

// ~(X + C) --> -(C + 1) - X
if (match(Op0, m_Add(m_Value(X), m_Constant(C))))
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Transforms/InstCombine/vector-xor.ll
Expand Up @@ -172,7 +172,7 @@ define <4 x i32> @test_v4i32_not_lshr_nonnegative_const(<4 x i32> %a0) {

define <4 x i32> @test_v4i32_not_lshr_nonnegative_const_undef(<4 x i32> %a0) {
; CHECK-LABEL: @test_v4i32_not_lshr_nonnegative_const_undef(
; CHECK-NEXT: [[TMP1:%.*]] = ashr <4 x i32> <i32 -4, i32 -6, i32 undef, i32 -10>, [[A0:%.*]]
; CHECK-NEXT: [[TMP1:%.*]] = ashr <4 x i32> <i32 -4, i32 -6, i32 -1, i32 -10>, [[A0:%.*]]
; CHECK-NEXT: ret <4 x i32> [[TMP1]]
;
%1 = lshr <4 x i32> <i32 3, i32 5, i32 undef, i32 9>, %a0
Expand Down

0 comments on commit 812970e

Please sign in to comment.