This is an archive of the discontinued LLVM Phabricator instance.

Differential D45538

[InstCombine][NFC]: Add tests: foldSelectICmpAndAnd(): and is commutative
ClosedPublic

Authored by lebedev.ri on Apr 11 2018, 3:22 PM.

Details

Reviewers
spatel
craig.topper
Commits
rG53271ba1d225: [InstCombine][NFC]: Add tests: foldSelectICmpAndAnd(): and is commutative
rL329901: [InstCombine][NFC]: Add tests: foldSelectICmpAndAnd(): and is commutative
Summary

The fold added in D45108 did not account for the fact that
the and instruction is commutative, and if the mask is a variable,
the mask variable and the fold variable may be swapped.

I have noticed this by accident when looking into PR6773

Diff Detail

Repository
rL LLVM

Event Timeline

lebedev.ri created this revision.Apr 11 2018, 3:22 PM
lebedev.ri added a child revision: D45539: [InstCombine]: foldSelectICmpAndAnd(): and is commutative.
spatel accepted this revision.Apr 11 2018, 4:02 PM

LGTM. I think you have a good understanding of how to write/check instcombine tests, so feel free to add those as needed without pre-commit review unless you're not sure about something.

This revision is now accepted and ready to land.Apr 11 2018, 4:02 PM
Closed by commit rL329901: [InstCombine][NFC]: Add tests: foldSelectICmpAndAnd(): and is commutative (authored by lebedevri). · Explain WhyApr 12 2018, 5:08 AM
This revision was automatically updated to reflect the committed changes.
lebedev.ri mentioned this in D45539: [InstCombine]: foldSelectICmpAndAnd(): and is commutative.Apr 12 2018, 1:45 PM

Revision Contents

PathSize
test/
Transforms/
InstCombine/
52 lines

Diff 142079

test/Transforms/InstCombine/select-of-bittest.ll

Show First 20 Lines • Show All 168 Lines • ▼ Show 20 Lines;
%tmp = and i32 %arg, %arg1 %tmp = and i32 %arg, %arg1
%tmp2 = icmp eq i32 %tmp, 0 %tmp2 = icmp eq i32 %tmp, 0
%tmp3 = lshr i32 %arg, 1 %tmp3 = lshr i32 %arg, 1
%tmp4 = and i32 %tmp3, 1 %tmp4 = and i32 %tmp3, 1
%tmp5 = select i1 %tmp2, i32 %tmp4, i32 1 %tmp5 = select i1 %tmp2, i32 %tmp4, i32 1
ret i32 %tmp5 ret i32 %tmp5
} }
; Should be exactly as the previous one
define i32 @f_var0_commutative_and(i32 %arg, i32 %arg1) {
; CHECK-LABEL: @f_var0_commutative_and(
; CHECK-NEXT: [[TMP:%.*]] = and i32 [[ARG1:%.*]], [[ARG:%.*]]
; CHECK-NEXT: [[TMP2:%.*]] = icmp eq i32 [[TMP]], 0
; CHECK-NEXT: [[TMP3:%.*]] = lshr i32 [[ARG]], 1
; CHECK-NEXT: [[TMP4:%.*]] = and i32 [[TMP3]], 1
; CHECK-NEXT: [[TMP5:%.*]] = select i1 [[TMP2]], i32 [[TMP4]], i32 1
; CHECK-NEXT: ret i32 [[TMP5]]
;
%tmp = and i32 %arg1, %arg ; in different order
%tmp2 = icmp eq i32 %tmp, 0
%tmp3 = lshr i32 %arg, 1
%tmp4 = and i32 %tmp3, 1
%tmp5 = select i1 %tmp2, i32 %tmp4, i32 1
ret i32 %tmp5
}
define <2 x i32> @f_var0_splatvec(<2 x i32> %arg, <2 x i32> %arg1) { define <2 x i32> @f_var0_splatvec(<2 x i32> %arg, <2 x i32> %arg1) {
; CHECK-LABEL: @f_var0_splatvec( ; CHECK-LABEL: @f_var0_splatvec(
; CHECK-NEXT: [[TMP1:%.*]] = or <2 x i32> [[ARG1:%.*]], <i32 2, i32 2> ; CHECK-NEXT: [[TMP1:%.*]] = or <2 x i32> [[ARG1:%.*]], <i32 2, i32 2>
; CHECK-NEXT: [[TMP2:%.*]] = and <2 x i32> [[TMP1]], [[ARG:%.*]] ; CHECK-NEXT: [[TMP2:%.*]] = and <2 x i32> [[TMP1]], [[ARG:%.*]]
; CHECK-NEXT: [[TMP3:%.*]] = icmp ne <2 x i32> [[TMP2]], zeroinitializer ; CHECK-NEXT: [[TMP3:%.*]] = icmp ne <2 x i32> [[TMP2]], zeroinitializer
; CHECK-NEXT: [[TMP5:%.*]] = zext <2 x i1> [[TMP3]] to <2 x i32> ; CHECK-NEXT: [[TMP5:%.*]] = zext <2 x i1> [[TMP3]] to <2 x i32>
; CHECK-NEXT: ret <2 x i32> [[TMP5]] ; CHECK-NEXT: ret <2 x i32> [[TMP5]]
; ;
▲ Show 20 Lines • Show All 48 Lines • ▼ Show 20 Lines
; ;
%tmp = and i32 %arg, %arg1 %tmp = and i32 %arg, %arg1
%tmp2 = icmp eq i32 %tmp, 0 %tmp2 = icmp eq i32 %tmp, 0
%tmp3 = and i32 %arg, 1 %tmp3 = and i32 %arg, 1
%tmp4 = select i1 %tmp2, i32 %tmp3, i32 1 %tmp4 = select i1 %tmp2, i32 %tmp3, i32 1
ret i32 %tmp4 ret i32 %tmp4
} }
; Should be exactly as the previous one
define i32 @f_var1_commutative_and(i32 %arg, i32 %arg1) {
; CHECK-LABEL: @f_var1_commutative_and(
; CHECK-NEXT: [[TMP:%.*]] = and i32 [[ARG1:%.*]], [[ARG:%.*]]
; CHECK-NEXT: [[TMP2:%.*]] = icmp eq i32 [[TMP]], 0
; CHECK-NEXT: [[TMP3:%.*]] = and i32 [[ARG]], 1
; CHECK-NEXT: [[TMP4:%.*]] = select i1 [[TMP2]], i32 [[TMP3]], i32 1
; CHECK-NEXT: ret i32 [[TMP4]]
;
%tmp = and i32 %arg1, %arg ; in different order
%tmp2 = icmp eq i32 %tmp, 0
%tmp3 = and i32 %arg, 1
%tmp4 = select i1 %tmp2, i32 %tmp3, i32 1
ret i32 %tmp4
}
define <2 x i32> @f_var1_vec(<2 x i32> %arg, <2 x i32> %arg1) { define <2 x i32> @f_var1_vec(<2 x i32> %arg, <2 x i32> %arg1) {
; CHECK-LABEL: @f_var1_vec( ; CHECK-LABEL: @f_var1_vec(
; CHECK-NEXT: [[TMP1:%.*]] = or <2 x i32> [[ARG1:%.*]], <i32 1, i32 1> ; CHECK-NEXT: [[TMP1:%.*]] = or <2 x i32> [[ARG1:%.*]], <i32 1, i32 1>
; CHECK-NEXT: [[TMP2:%.*]] = and <2 x i32> [[TMP1]], [[ARG:%.*]] ; CHECK-NEXT: [[TMP2:%.*]] = and <2 x i32> [[TMP1]], [[ARG:%.*]]
; CHECK-NEXT: [[TMP3:%.*]] = icmp ne <2 x i32> [[TMP2]], zeroinitializer ; CHECK-NEXT: [[TMP3:%.*]] = icmp ne <2 x i32> [[TMP2]], zeroinitializer
; CHECK-NEXT: [[TMP4:%.*]] = zext <2 x i1> [[TMP3]] to <2 x i32> ; CHECK-NEXT: [[TMP4:%.*]] = zext <2 x i1> [[TMP3]] to <2 x i32>
; CHECK-NEXT: ret <2 x i32> [[TMP4]] ; CHECK-NEXT: ret <2 x i32> [[TMP4]]
; ;
▲ Show 20 Lines • Show All 107 Lines • ▼ Show 20 Lines;
%tmp = and i32 %arg, %arg1 %tmp = and i32 %arg, %arg1
%tmp3 = icmp eq i32 %tmp, 0 %tmp3 = icmp eq i32 %tmp, 0
%tmp4 = lshr i32 %arg, %arg2 %tmp4 = lshr i32 %arg, %arg2
%tmp5 = and i32 %tmp4, 1 %tmp5 = and i32 %tmp4, 1
%tmp6 = select i1 %tmp3, i32 %tmp5, i32 1 %tmp6 = select i1 %tmp3, i32 %tmp5, i32 1
ret i32 %tmp6 ret i32 %tmp6
} }
; Should be exactly as the previous one
define i32 @f_var3_commutative_and(i32 %arg, i32 %arg1, i32 %arg2) {
; CHECK-LABEL: @f_var3_commutative_and(
; CHECK-NEXT: [[TMP:%.*]] = and i32 [[ARG1:%.*]], [[ARG:%.*]]
; CHECK-NEXT: [[TMP3:%.*]] = icmp eq i32 [[TMP]], 0
; CHECK-NEXT: [[TMP4:%.*]] = lshr i32 [[ARG]], [[ARG2:%.*]]
; CHECK-NEXT: [[TMP5:%.*]] = and i32 [[TMP4]], 1
; CHECK-NEXT: [[TMP6:%.*]] = select i1 [[TMP3]], i32 [[TMP5]], i32 1
; CHECK-NEXT: ret i32 [[TMP6]]
;
%tmp = and i32 %arg1, %arg ; in different order
%tmp3 = icmp eq i32 %tmp, 0
%tmp4 = lshr i32 %arg, %arg2
%tmp5 = and i32 %tmp4, 1
%tmp6 = select i1 %tmp3, i32 %tmp5, i32 1
ret i32 %tmp6
}
define <2 x i32> @f_var3_splatvec(<2 x i32> %arg, <2 x i32> %arg1, <2 x i32> %arg2) { define <2 x i32> @f_var3_splatvec(<2 x i32> %arg, <2 x i32> %arg1, <2 x i32> %arg2) {
; CHECK-LABEL: @f_var3_splatvec( ; CHECK-LABEL: @f_var3_splatvec(
; CHECK-NEXT: [[TMP1:%.*]] = shl <2 x i32> <i32 1, i32 1>, [[ARG2:%.*]] ; CHECK-NEXT: [[TMP1:%.*]] = shl <2 x i32> <i32 1, i32 1>, [[ARG2:%.*]]
; CHECK-NEXT: [[TMP2:%.*]] = or <2 x i32> [[TMP1]], [[ARG1:%.*]] ; CHECK-NEXT: [[TMP2:%.*]] = or <2 x i32> [[TMP1]], [[ARG1:%.*]]
; CHECK-NEXT: [[TMP3:%.*]] = and <2 x i32> [[TMP2]], [[ARG:%.*]] ; CHECK-NEXT: [[TMP3:%.*]] = and <2 x i32> [[TMP2]], [[ARG:%.*]]
; CHECK-NEXT: [[TMP4:%.*]] = icmp ne <2 x i32> [[TMP3]], zeroinitializer ; CHECK-NEXT: [[TMP4:%.*]] = icmp ne <2 x i32> [[TMP3]], zeroinitializer
; CHECK-NEXT: [[TMP6:%.*]] = zext <2 x i1> [[TMP4]] to <2 x i32> ; CHECK-NEXT: [[TMP6:%.*]] = zext <2 x i1> [[TMP4]] to <2 x i32>
; CHECK-NEXT: ret <2 x i32> [[TMP6]] ; CHECK-NEXT: ret <2 x i32> [[TMP6]]
▲ Show 20 Lines • Show All 236 Lines • Show Last 20 Lines