Skip to content

Commit

Permalink
[InstCombine] Fix incorrect zero ext in select of lshr/ashr fold
Browse files Browse the repository at this point in the history
The -1 constant should be sign extended, not zero extended.

Split out from #80309.
  • Loading branch information
nikic committed Aug 16, 2024
1 parent ce128d8 commit 5d28678
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 9 deletions.
8 changes: 4 additions & 4 deletions llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -679,11 +679,11 @@ static Value *foldSelectICmpLshrAshr(const ICmpInst *IC, Value *TrueVal,
Value *X, *Y;
unsigned Bitwidth = CmpRHS->getType()->getScalarSizeInBits();
if ((Pred != ICmpInst::ICMP_SGT ||
!match(CmpRHS,
m_SpecificInt_ICMP(ICmpInst::ICMP_SGE, APInt(Bitwidth, -1)))) &&
!match(CmpRHS, m_SpecificInt_ICMP(ICmpInst::ICMP_SGE,
APInt::getAllOnes(Bitwidth)))) &&
(Pred != ICmpInst::ICMP_SLT ||
!match(CmpRHS,
m_SpecificInt_ICMP(ICmpInst::ICMP_SGE, APInt(Bitwidth, 0)))))
!match(CmpRHS, m_SpecificInt_ICMP(ICmpInst::ICMP_SGE,
APInt::getZero(Bitwidth)))))
return nullptr;

// Canonicalize so that ashr is in FalseVal.
Expand Down
7 changes: 2 additions & 5 deletions llvm/test/Transforms/InstCombine/ashr-lshr.ll
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,8 @@ define i32 @ashr_lshr2(i32 %x, i32 %y) {

define i128 @ashr_lshr2_i128(i128 %x, i128 %y) {
; CHECK-LABEL: @ashr_lshr2_i128(
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i128 [[X:%.*]], 5
; CHECK-NEXT: [[L:%.*]] = lshr i128 [[X]], [[Y:%.*]]
; CHECK-NEXT: [[R:%.*]] = ashr exact i128 [[X]], [[Y]]
; CHECK-NEXT: [[RET:%.*]] = select i1 [[CMP]], i128 [[L]], i128 [[R]]
; CHECK-NEXT: ret i128 [[RET]]
; CHECK-NEXT: [[CMP1:%.*]] = ashr i128 [[X:%.*]], [[Y:%.*]]
; CHECK-NEXT: ret i128 [[CMP1]]
;
%cmp = icmp sgt i128 %x, 5
%l = lshr i128 %x, %y
Expand Down

2 comments on commit 5d28678

@DTeachs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this fixing a correctness issue or is an enhacement? If it is a correctness issue, can we backport to 19.x?

@nikic
Copy link
Contributor Author

@nikic nikic commented on 5d28678 Aug 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is not a correctness issue, only a missed optimization.

Please sign in to comment.