Skip to content

Commit

Permalink
[InstCombine] fold X urem C -> X < C ? X : X - C when C is big (PR28672)
Browse files Browse the repository at this point in the history
We already have the udiv variant of this transform, so I think this is ok for 
InstCombine too even though there is an increase in IR instructions. As the 
tests and TODO comments show, the transform can lead to follow-on combines.

This should fix: https://llvm.org/bugs/show_bug.cgi?id=28672

Differential Revision: https://reviews.llvm.org/D24527

llvm-svn: 282209
  • Loading branch information
rotateright committed Sep 22, 2016
1 parent 3acdf38 commit 30ef70b
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 3 deletions.
8 changes: 8 additions & 0 deletions llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
Expand Up @@ -1447,6 +1447,14 @@ Instruction *InstCombiner::visitURem(BinaryOperator &I) {
return replaceInstUsesWith(I, Ext);
}

// X urem C -> X < C ? X : X - C, where C >= signbit.
const APInt *DivisorC;
if (match(Op1, m_APInt(DivisorC)) && DivisorC->isNegative()) {
Value *Cmp = Builder->CreateICmpULT(Op0, Op1);
Value *Sub = Builder->CreateSub(Op0, Op1);
return SelectInst::Create(Cmp, Op0, Sub);
}

return nullptr;
}

Expand Down
14 changes: 11 additions & 3 deletions llvm/test/Transforms/InstCombine/urem.ll
Expand Up @@ -16,25 +16,33 @@ define i64 @rem_unsigned(i64 %x1, i64 %y2) {

define i8 @big_divisor(i8 %x) {
; CHECK-LABEL: @big_divisor(
; CHECK-NEXT: [[REM:%.*]] = urem i8 %x, -127
; CHECK-NEXT: [[TMP1:%.*]] = icmp ult i8 %x, -127
; CHECK-NEXT: [[TMP2:%.*]] = add i8 %x, 127
; CHECK-NEXT: [[REM:%.*]] = select i1 [[TMP1]], i8 %x, i8 [[TMP2]]
; CHECK-NEXT: ret i8 [[REM]]
;
%rem = urem i8 %x, 129
ret i8 %rem
}

; TODO: Should this be zext+add instead of sext+sub?
define i5 @biggest_divisor(i5 %x) {
; CHECK-LABEL: @biggest_divisor(
; CHECK-NEXT: [[REM:%.*]] = urem i5 %x, -1
; CHECK-NEXT: [[NOT_:%.*]] = icmp eq i5 %x, -1
; CHECK-NEXT: [[TMP1:%.*]] = sext i1 [[NOT_]] to i5
; CHECK-NEXT: [[REM:%.*]] = sub i5 %x, [[TMP1]]
; CHECK-NEXT: ret i5 [[REM]]
;
%rem = urem i5 %x, -1
ret i5 %rem
}

; TODO: Should vector subtract of constant be canonicalized to add?
define <2 x i4> @big_divisor_vec(<2 x i4> %x) {
; CHECK-LABEL: @big_divisor_vec(
; CHECK-NEXT: [[REM:%.*]] = urem <2 x i4> %x, <i4 -3, i4 -3>
; CHECK-NEXT: [[TMP1:%.*]] = icmp ult <2 x i4> %x, <i4 -3, i4 -3>
; CHECK-NEXT: [[TMP2:%.*]] = sub <2 x i4> %x, <i4 -3, i4 -3>
; CHECK-NEXT: [[REM:%.*]] = select <2 x i1> [[TMP1]], <2 x i4> %x, <2 x i4> [[TMP2]]
; CHECK-NEXT: ret <2 x i4> [[REM]]
;
%rem = urem <2 x i4> %x, <i4 13, i4 13>
Expand Down

0 comments on commit 30ef70b

Please sign in to comment.