This is an archive of the discontinued LLVM Phabricator instance.

Differential D75906

[InstCombine] Support vectors in SimplifyAddWithRemainder.
ClosedPublic

Authored by fhahn on Mar 10 2020, 4:27 AM.

Details

Reviewers
spatel
lebedev.ri
Commits
rG7304fdef0235: [InstCombine] Support vectors in SimplifyAddWithRemainder.
rGc8c14d979abc: [InstCombine] Support vectors in SimplifyAddWithRemainder.
Summary

SimplifyAddWithRemainder currently also matches for vector types, but
tries to create an integer constant, which causes a crash.

By using Constant::getIntegerValue() we can support both the scalar and
vector cases.

The 2 added test cases crash without the fix.

Diff Detail

Event Timeline

fhahn created this revision.Mar 10 2020, 4:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 10 2020, 4:27 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
lebedev.ri added inline comments.Mar 10 2020, 4:37 AM
llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
1048

I think you want ConstantInt::get(X->getType(), C0 * C1)

Harbormaster failed remote builds in B48657: Diff 249311!Mar 10 2020, 5:12 AM
fhahn updated this revision to Diff 249341.Mar 10 2020, 5:53 AM

Fix clang-format error

fhahn updated this revision to Diff 249343.Mar 10 2020, 5:57 AM

use ConstantInt::get().

fhahn marked an inline comment as done.Mar 10 2020, 5:58 AM
fhahn added inline comments.
llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
1048

I think they should do the same thing. ConstantInt::get() is a bit shorter though, I've updated the code.

lebedev.ri accepted this revision.Mar 10 2020, 6:30 AM

LG

This revision is now accepted and ready to land.Mar 10 2020, 6:30 AM
Harbormaster completed remote builds in B48673: Diff 249343.Mar 10 2020, 7:00 AM
spatel accepted this revision.Mar 10 2020, 7:02 AM

LGTM, but you'll need to rebase the test diffs because I regenerated the test file to avoid a bug in utils/update_test_checks.py:
rG6e60e1025f32

Harbormaster completed remote builds in B48672: Diff 249341.Mar 10 2020, 7:32 AM
Closed by commit rGc8c14d979abc: [InstCombine] Support vectors in SimplifyAddWithRemainder. (authored by fhahn). · Explain WhyMar 10 2020, 7:34 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
llvm/
lib/
Transforms/
InstCombine/
2 lines
test/
Transforms/
InstCombine/
33 lines

Diff 249311

llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp

Show First 20 Lines • Show All 1,038 Lines • ▼ Show 20 Lines if (((MatchRem(LHS, X, C0, IsSigned) && MatchMul(RHS, MulOpV, MulOpC)) ||
// Match MulOpC = RemOpV % C1 // Match MulOpC = RemOpV % C1
if (MatchRem(MulOpV, RemOpV, C1, Rem2IsSigned) && if (MatchRem(MulOpV, RemOpV, C1, Rem2IsSigned) &&
IsSigned == Rem2IsSigned) { IsSigned == Rem2IsSigned) {
Value *DivOpV; Value *DivOpV;
APInt DivOpC; APInt DivOpC;
// Match RemOpV = X / C0 // Match RemOpV = X / C0
if (MatchDiv(RemOpV, DivOpV, DivOpC, IsSigned) && X == DivOpV && if (MatchDiv(RemOpV, DivOpV, DivOpC, IsSigned) && X == DivOpV &&
C0 == DivOpC && !MulWillOverflow(C0, C1, IsSigned)) { C0 == DivOpC && !MulWillOverflow(C0, C1, IsSigned)) {
Value *NewDivisor = Value *NewDivisor =
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-        Value *NewDivisor =
-           Constant::getIntegerValue(X->getType(), C0 * C1);
+        Value *NewDivisor = Constant::getIntegerValue(X->getType(), C0 * C1);
Lint: Pre-merge checks: clang-format: please reformat the code ``` - Value *NewDivisor = - Constant…
ConstantInt::get(X->getType()->getContext(), C0 * C1); Constant::getIntegerValue(X->getType(), C0 * C1);
lebedev.riUnsubmitted
Not Done
ReplyInline Actions

I think you want ConstantInt::get(X->getType(), C0 * C1)

lebedev.ri: I think you want `ConstantInt::get(X->getType(), C0 * C1)`
fhahnAuthorUnsubmitted
Done
ReplyInline Actions

I think they should do the same thing. ConstantInt::get() is a bit shorter though, I've updated the code.

fhahn: I think they should do the same thing. ConstantInt::get() is a bit shorter though, I've updated…
return IsSigned ? Builder.CreateSRem(X, NewDivisor, "srem") return IsSigned ? Builder.CreateSRem(X, NewDivisor, "srem")
: Builder.CreateURem(X, NewDivisor, "urem"); : Builder.CreateURem(X, NewDivisor, "urem");
} }
} }
} }
return nullptr; return nullptr;
} }
▲ Show 20 Lines • Show All 1,198 Lines • Show Last 20 Lines

llvm/test/Transforms/InstCombine/add4.ll

Show All 12 Linesbb:
%tmp = urem i64 %x, 299 %tmp = urem i64 %x, 299
%tmp1 = udiv i64 %x, 299 %tmp1 = udiv i64 %x, 299
%tmp2 = urem i64 %tmp1, 64 %tmp2 = urem i64 %tmp1, 64
%tmp3 = mul i64 %tmp2, 299 %tmp3 = mul i64 %tmp2, 299
%tmp4 = add i64 %tmp, %tmp3 %tmp4 = add i64 %tmp, %tmp3
ret i64 %tmp4 ret i64 %tmp4
} }
define <2 x i64> @match_unsigned_vector(<2 x i64> %x) {
; CHECK-LABEL: @match_unsigned_vector(
; CHECK-NEXT: bb:
; CHECK-NEXT: [[UREM:%.*]] = urem <2 x i64> [[X:%.*]], <i64 19136, i64 19136>
; CHECK-NEXT: ret <2 x i64> [[UREM]]
;
bb:
%tmp = urem <2 x i64> %x, <i64 299, i64 299>
%tmp1 = udiv <2 x i64> %x, <i64 299, i64 299>
%tmp2 = urem <2 x i64> %tmp1, <i64 64, i64 64>
%tmp3 = mul <2 x i64> %tmp2, <i64 299, i64 299>
%tmp4 = add <2 x i64> %tmp, %tmp3
ret <2 x i64> %tmp4
}
define i64 @match_andAsRem_lshrAsDiv_shlAsMul(i64 %x) { define i64 @match_andAsRem_lshrAsDiv_shlAsMul(i64 %x) {
; CHECK-LABEL: @match_andAsRem_lshrAsDiv_shlAsMul( ; CHECK-LABEL: @match_andAsRem_lshrAsDiv_shlAsMul(
; CHECK-NEXT: bb: ; CHECK-NEXT: bb:
; CHECK-NEXT: [[UREM:%.*]] = urem i64 [[X:%.*]], 576 ; CHECK-NEXT: [[UREM:%.*]] = urem i64 [[X:%.*]], 576
; CHECK-NEXT: ret i64 [[UREM]] ; CHECK-NEXT: ret i64 [[UREM]]
; ;
bb: bb:
%tmp = and i64 %x, 63 %tmp = and i64 %x, 63
Show All 18 Linesbb:
%tmp4 = srem i64 %tmp3, 9 %tmp4 = srem i64 %tmp3, 9
%tmp5 = mul i64 %tmp2, 299 %tmp5 = mul i64 %tmp2, 299
%tmp6 = add i64 %tmp, %tmp5 %tmp6 = add i64 %tmp, %tmp5
%tmp7 = mul i64 %tmp4, 19136 %tmp7 = mul i64 %tmp4, 19136
%tmp8 = add i64 %tmp6, %tmp7 %tmp8 = add i64 %tmp6, %tmp7
ret i64 %tmp8 ret i64 %tmp8
} }
define <2 x i64> @match_signed_vector(<2 x i64> %x) {
; CHECK-LABEL: @match_signed_vector(
; CHECK-NEXT: bb:
; CHECK-NEXT: [[SREM1:%.*]] = srem <2 x i64> [[X:%.*]], <i64 172224, i64 172224>
; CHECK-NEXT: ret <2 x i64> [[SREM1]]
;
bb:
%tmp = srem <2 x i64> %x, <i64 299, i64 299>
%tmp1 = sdiv <2 x i64> %x, <i64 299, i64 299>
%tmp2 = srem <2 x i64> %tmp1, <i64 64, i64 64>
%tmp3 = sdiv <2 x i64> %x, <i64 19136, i64 19136>
%tmp4 = srem <2 x i64> %tmp3, <i64 9, i64 9>
%tmp5 = mul <2 x i64> %tmp2, <i64 299, i64 299>
%tmp6 = add <2 x i64> %tmp, %tmp5
%tmp7 = mul <2 x i64> %tmp4, <i64 19136, i64 19136>
%tmp8 = add <2 x i64> %tmp6, %tmp7
ret <2 x i64> %tmp8
}
define i64 @not_match_inconsistent_signs(i64 %x) { define i64 @not_match_inconsistent_signs(i64 %x) {
; CHECK-LABEL: @not_match_inconsistent_signs( ; CHECK-LABEL: @not_match_inconsistent_signs(
; CHECK-NEXT: bb: ; CHECK-NEXT: bb:
; CHECK-NEXT: [[TMP:%.*]] = urem i64 [[X:%.*]], 299 ; CHECK-NEXT: [[TMP:%.*]] = urem i64 [[X:%.*]], 299
; CHECK-NEXT: [[TMP1:%.*]] = sdiv i64 [[X]], 299 ; CHECK-NEXT: [[TMP1:%.*]] = sdiv i64 [[X]], 299
; CHECK-NEXT: [[TMP2:%.*]] = and i64 [[TMP1]], 63 ; CHECK-NEXT: [[TMP2:%.*]] = and i64 [[TMP1]], 63
; CHECK-NEXT: [[TMP3:%.*]] = mul nuw nsw i64 [[TMP2]], 299 ; CHECK-NEXT: [[TMP3:%.*]] = mul nuw nsw i64 [[TMP2]], 299
; CHECK-NEXT: [[TMP4:%.*]] = add nuw nsw i64 [[TMP]], [[TMP3]] ; CHECK-NEXT: [[TMP4:%.*]] = add nuw nsw i64 [[TMP]], [[TMP3]]
▲ Show 20 Lines • Show All 47 Lines • Show Last 20 Lines