[InstCombine] Add bswap(logic_op(bswap(x), y)) regression test case; NFC

[austin880625]

Summary

Fold the following case on IR InstCombine pass. This patch includes the new test cases for this optimization

bswap(logic_op(x, bswap(y))) -> logic_op(bswap(x), y)
bswap(logic_op(bswap(x), y)) -> logic_op(x, bswap(y))
bswap(logic_op(bswap(x), bswap(y))) -> logic_op(x, y) with multi-use

[goldstein.w.n]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

1693 ↗

(On Diff #518420)

I don't see how this is correct if we only have 1 bswap:
https://alive2.llvm.org/ce/z/zBErQk

Also this apples for bitreverse as well.

llvm/test/Transforms/InstCombine/bswap-fold.ll

542

Can you split the new tests to a seperate patch so we can see the diff generated?

[goldstein.w.n]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

1632 ↗

(On Diff #518420)

nit, but since we already have the scope for ::bswap case, can you just declare V before the match.

[goldstein.w.n]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

1682 ↗

(On Diff #518420)

Can you move this to a helper function that takes an Intrisinic ID so we can reuse for bitreverse?

1693 ↗

(On Diff #518420)

I don't see how this is correct if we only have 1 bswap:
https://alive2.llvm.org/ce/z/zBErQk

Also this apples for bitreverse as well.

Nevermind, saw this as standalone, missed it was in the bswap case.

[austin880625]

hi, thanks for the reviewing! I'll keep working on the suggestions and update here once I've done(probably in 2-3 days)

[austin880625]

llvm/test/Transforms/InstCombine/bswap-fold.ll

542

Just to confirm(newbie here), are the following separated patches you want:

1. first one in old implementation and new tests with FileCheck lines generated with old opt
2. second one with my actual code change, and the new tests with updated FileCheck lines generated with the changed opt. And the patch is based diff-ed with the first one?

[goldstein.w.n]

llvm/test/Transforms/InstCombine/bswap-fold.ll

542

Just to confirm(newbie here), are the following separated patches you want:

1. first one in old implementation and new tests with FileCheck lines generated with old opt
2. second one with my actual code change, and the new tests with updated FileCheck lines generated with the changed opt. And the patch is based diff-ed with the first one?

Yeah. You can create series using the 'edit related revisions' then make the test-patch the "parent".
For example:
Tests: D149520
Impl: D149521

and if click 'stack' you see the relationship between the two.

542

Just to confirm(newbie here), are the following separated patches you want:

1. first one in old implementation and new tests with FileCheck lines generated with old opt
2. second one with my actual code change, and the new tests with updated FileCheck lines generated with the changed opt. And the patch is based diff-ed with the first one?

Yeah. You can create series using the 'edit related revisions' then make the test-patch the "parent".
For example:
Tests: D149520
Impl: D149521

and if click 'stack' you see the relationship between the two.

542

And welcome! Thank you for the patch!

[austin880625]

separate test and implementation

[austin880625]

@goldstein.w.n I updated the diff on this revision (as test) and created another D149699 as updated implementation. Let me know if I did anything wrong here, thanks!

[goldstein.w.n]

In D149577#4313910, @austin880625 wrote:

@goldstein.w.n I updated the diff on this revision (as test) and created another D149699 as updated implementation. Let me know if I did anything wrong here, thanks!

You created the series properly.

Can you update the summary/title of the two commits to reflect what they are now.

So D149577 doesn't need a summary but the title should be something along the lines of:
"[InstCombine] Add tests for (logic_op (bswap X), Y); NFC"

D149699 should have basically the same title/commit as you originally had (as you have here now).

[austin880625]

add test cases for CodeGen(DAGCombiner)

[austin880625]

contains changes for InstCombine only

[goldstein.w.n]

llvm/test/Transforms/InstCombine/bswap-fold.ll

781

your *_rhs_* tests appear to have the bswap still on the lhs.

[austin880625]

add bswap(logic_op(bswap(x), bswap(y))) -> logic_op(x, y) case with multi-use

[goldstein.w.n]

LGTM.

[RKSimon]

LGTM

[RKSimon]

@austin880625 Do you have commit access, if not please can you post your github email address so I can push the changes for you

[austin880625]

In D149577#4324950, @RKSimon wrote:

@austin880625 Do you have commit access, if not please can you post your github email address so I can push the changes for you

@RKSimon I don't, can you use this one: [e-mail] . Thanks!

llvm/test/Transforms/InstCombine/bswap-fold.ll

 ;
   %t1 = tail call i64 @llvm.bswap.i64(i64 %a)
   %t2 = and i64 %t1, 1000000001
   %t3 = mul i64 %t2, %t1 ; to increase use count of the bswap
   ret i64 %t3
 }
 
 
+; Issue#62236
+; Fold: BSWAP( OP( BSWAP(x), y ) ) -> OP( x, BSWAP(y) )
+define i16 @bs_and_lhs_bs16(i16 %a, i16 %b) #0 {
+; CHECK-LABEL: @bs_and_lhs_bs16(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call i16 @llvm.bswap.i16(i16 [[A:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = and i16 [[TMP1]], [[B:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call i16 @llvm.bswap.i16(i16 [[TMP2]])
+; CHECK-NEXT:    ret i16 [[TMP3]]
+;
+  %1 = tail call i16 @llvm.bswap.i16(i16 %a)
+  %2 = and i16 %1, %b
+  %3 = tail call i16 @llvm.bswap.i16(i16 %2)
+  ret i16 %3
+}
+
+define i16 @bs_or_lhs_bs16(i16 %a, i16 %b) #0 {
+; CHECK-LABEL: @bs_or_lhs_bs16(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call i16 @llvm.bswap.i16(i16 [[A:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = or i16 [[TMP1]], [[B:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call i16 @llvm.bswap.i16(i16 [[TMP2]])
+; CHECK-NEXT:    ret i16 [[TMP3]]
+;
+  %1 = tail call i16 @llvm.bswap.i16(i16 %a)
+  %2 = or i16 %1, %b
+  %3 = tail call i16 @llvm.bswap.i16(i16 %2)
+  ret i16 %3
+}
+
+define i16 @bs_xor_lhs_bs16(i16 %a, i16 %b) #0 {
+; CHECK-LABEL: @bs_xor_lhs_bs16(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call i16 @llvm.bswap.i16(i16 [[A:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = xor i16 [[TMP1]], [[B:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call i16 @llvm.bswap.i16(i16 [[TMP2]])
+; CHECK-NEXT:    ret i16 [[TMP3]]
+;
+  %1 = tail call i16 @llvm.bswap.i16(i16 %a)
+  %2 = xor i16 %1, %b
+  %3 = tail call i16 @llvm.bswap.i16(i16 %2)
+  ret i16 %3
+}
+
+define i16 @bs_and_rhs_bs16(i16 %a, i16 %b) #0 {
+; CHECK-LABEL: @bs_and_rhs_bs16(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call i16 @llvm.bswap.i16(i16 [[B:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = and i16 [[TMP1]], [[A:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call i16 @llvm.bswap.i16(i16 [[TMP2]])
+; CHECK-NEXT:    ret i16 [[TMP3]]
+;
+  %1 = tail call i16 @llvm.bswap.i16(i16 %b)
+  %2 = and i16 %a, %1
+  %3 = tail call i16 @llvm.bswap.i16(i16 %2)
+  ret i16 %3
+}
+
+define i16 @bs_or_rhs_bs16(i16 %a, i16 %b) #0 {
+; CHECK-LABEL: @bs_or_rhs_bs16(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call i16 @llvm.bswap.i16(i16 [[B:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = or i16 [[TMP1]], [[A:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call i16 @llvm.bswap.i16(i16 [[TMP2]])
+; CHECK-NEXT:    ret i16 [[TMP3]]
+;
+  %1 = tail call i16 @llvm.bswap.i16(i16 %b)
+  %2 = or i16 %a, %1
+  %3 = tail call i16 @llvm.bswap.i16(i16 %2)
+  ret i16 %3
+}
+
+define i16 @bs_xor_rhs_bs16(i16 %a, i16 %b) #0 {
+; CHECK-LABEL: @bs_xor_rhs_bs16(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call i16 @llvm.bswap.i16(i16 [[B:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = xor i16 [[TMP1]], [[A:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call i16 @llvm.bswap.i16(i16 [[TMP2]])
+; CHECK-NEXT:    ret i16 [[TMP3]]
+;
+  %1 = tail call i16 @llvm.bswap.i16(i16 %b)
+  %2 = xor i16 %a, %1
+  %3 = tail call i16 @llvm.bswap.i16(i16 %2)
+  ret i16 %3
+}
+
+define i32 @bs_and_rhs_bs32(i32 %a, i32 %b) #0 {
+; CHECK-LABEL: @bs_and_rhs_bs32(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call i32 @llvm.bswap.i32(i32 [[B:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = and i32 [[TMP1]], [[A:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call i32 @llvm.bswap.i32(i32 [[TMP2]])
+; CHECK-NEXT:    ret i32 [[TMP3]]
+;
+  %1 = tail call i32 @llvm.bswap.i32(i32 %b)
+  %2 = and i32 %a, %1
+  %3 = tail call i32 @llvm.bswap.i32(i32 %2)
+  ret i32 %3
+}
+
+define i32 @bs_or_rhs_bs32(i32 %a, i32 %b) #0 {
+; CHECK-LABEL: @bs_or_rhs_bs32(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call i32 @llvm.bswap.i32(i32 [[B:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = or i32 [[TMP1]], [[A:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call i32 @llvm.bswap.i32(i32 [[TMP2]])
+; CHECK-NEXT:    ret i32 [[TMP3]]
+;
+  %1 = tail call i32 @llvm.bswap.i32(i32 %b)
+  %2 = or i32 %a, %1
+  %3 = tail call i32 @llvm.bswap.i32(i32 %2)
+  ret i32 %3
+}
+
+define i32 @bs_xor_rhs_bs32(i32 %a, i32 %b) #0 {
+; CHECK-LABEL: @bs_xor_rhs_bs32(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call i32 @llvm.bswap.i32(i32 [[B:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = xor i32 [[TMP1]], [[A:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call i32 @llvm.bswap.i32(i32 [[TMP2]])
+; CHECK-NEXT:    ret i32 [[TMP3]]
+;
+  %1 = tail call i32 @llvm.bswap.i32(i32 %b)
+  %2 = xor i32 %a, %1
+  %3 = tail call i32 @llvm.bswap.i32(i32 %2)
+  ret i32 %3
+}
+
+define i64 @bs_and_rhs_bs64(i64 %a, i64 %b) #0 {
+; CHECK-LABEL: @bs_and_rhs_bs64(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call i64 @llvm.bswap.i64(i64 [[B:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = and i64 [[TMP1]], [[A:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call i64 @llvm.bswap.i64(i64 [[TMP2]])
+; CHECK-NEXT:    ret i64 [[TMP3]]
+;
+  %1 = tail call i64 @llvm.bswap.i64(i64 %b)
+  %2 = and i64 %a, %1
+  %3 = tail call i64 @llvm.bswap.i64(i64 %2)
+  ret i64 %3
+}
+
+define i64 @bs_or_rhs_bs64(i64 %a, i64 %b) #0 {
+; CHECK-LABEL: @bs_or_rhs_bs64(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call i64 @llvm.bswap.i64(i64 [[B:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = or i64 [[TMP1]], [[A:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call i64 @llvm.bswap.i64(i64 [[TMP2]])
+; CHECK-NEXT:    ret i64 [[TMP3]]
+;
+  %1 = tail call i64 @llvm.bswap.i64(i64 %b)
+  %2 = or i64 %a, %1
+  %3 = tail call i64 @llvm.bswap.i64(i64 %2)
+  ret i64 %3
+}
+
+define i64 @bs_xor_rhs_bs64(i64 %a, i64 %b) #0 {
+; CHECK-LABEL: @bs_xor_rhs_bs64(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call i64 @llvm.bswap.i64(i64 [[B:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = xor i64 [[TMP1]], [[A:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call i64 @llvm.bswap.i64(i64 [[TMP2]])
+; CHECK-NEXT:    ret i64 [[TMP3]]
+;
+  %1 = tail call i64 @llvm.bswap.i64(i64 %b)
+  %2 = xor i64 %a, %1
+  %3 = tail call i64 @llvm.bswap.i64(i64 %2)
+  ret i64 %3
+}
+
+define <2 x i32> @bs_and_rhs_i32vec(<2 x i32> %a, <2 x i32> %b) #0 {
+; CHECK-LABEL: @bs_and_rhs_i32vec(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call <2 x i32> @llvm.bswap.v2i32(<2 x i32> [[B:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = and <2 x i32> [[TMP1]], [[A:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call <2 x i32> @llvm.bswap.v2i32(<2 x i32> [[TMP2]])
+; CHECK-NEXT:    ret <2 x i32> [[TMP3]]
+;
+  %1 = tail call <2 x i32> @llvm.bswap.v2i32(<2 x i32> %b)
+  %2 = and <2 x i32> %a, %1
+  %3 = tail call <2 x i32> @llvm.bswap.v2i32(<2 x i32> %2)
+  ret <2 x i32> %3
+}
+
+define <2 x i32> @bs_or_rhs_i32vec(<2 x i32> %a, <2 x i32> %b) #0 {
+; CHECK-LABEL: @bs_or_rhs_i32vec(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call <2 x i32> @llvm.bswap.v2i32(<2 x i32> [[B:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = or <2 x i32> [[TMP1]], [[A:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call <2 x i32> @llvm.bswap.v2i32(<2 x i32> [[TMP2]])
+; CHECK-NEXT:    ret <2 x i32> [[TMP3]]
+;
+  %1 = tail call <2 x i32> @llvm.bswap.v2i32(<2 x i32> %b)
+  %2 = or <2 x i32> %a, %1
+  %3 = tail call <2 x i32> @llvm.bswap.v2i32(<2 x i32> %2)
+  ret <2 x i32> %3
+}
+
+define <2 x i32> @bs_xor_rhs_i32vec(<2 x i32> %a, <2 x i32> %b) #0 {
+; CHECK-LABEL: @bs_xor_rhs_i32vec(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call <2 x i32> @llvm.bswap.v2i32(<2 x i32> [[B:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = xor <2 x i32> [[TMP1]], [[A:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call <2 x i32> @llvm.bswap.v2i32(<2 x i32> [[TMP2]])
+; CHECK-NEXT:    ret <2 x i32> [[TMP3]]
+;
+  %1 = tail call <2 x i32> @llvm.bswap.v2i32(<2 x i32> %b)
+  %2 = xor <2 x i32> %a, %1
+  %3 = tail call <2 x i32> @llvm.bswap.v2i32(<2 x i32> %2)
+  ret <2 x i32> %3
+}
+
+define i64 @bs_and_rhs_bs64_multiuse1(i64 %a, i64 %b) #0 {
+; CHECK-LABEL: @bs_and_rhs_bs64_multiuse1(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call i64 @llvm.bswap.i64(i64 [[B:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = and i64 [[TMP1]], [[A:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call i64 @llvm.bswap.i64(i64 [[TMP2]])
+; CHECK-NEXT:    [[TMP4:%.*]] = mul i64 [[TMP2]], [[TMP3]]
+; CHECK-NEXT:    ret i64 [[TMP4]]
+;
+  %1 = tail call i64 @llvm.bswap.i64(i64 %b)
+  %2 = and i64 %a, %1
+  %3 = tail call i64 @llvm.bswap.i64(i64 %2)
+  %4 = mul i64 %2, %3 ;increase use of logical op
+  ret i64 %4
+}
+
+define i64 @bs_and_rhs_bs64_multiuse2(i64 %a, i64 %b) #0 {
+; CHECK-LABEL: @bs_and_rhs_bs64_multiuse2(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call i64 @llvm.bswap.i64(i64 [[B:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = and i64 [[TMP1]], [[A:%.*]]
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call i64 @llvm.bswap.i64(i64 [[TMP2]])
+; CHECK-NEXT:    [[TMP4:%.*]] = mul i64 [[TMP1]], [[TMP3]]
+; CHECK-NEXT:    ret i64 [[TMP4]]
+;
+  %1 = tail call i64 @llvm.bswap.i64(i64 %b)
+  %2 = and i64 %a, %1
+  %3 = tail call i64 @llvm.bswap.i64(i64 %2)
+  %4 = mul i64 %1, %3 ;increase use of inner bswap
+  ret i64 %4
+}
+
+define i64 @bs_all_operand64(i64 %a, i64 %b) #0 {
+; CHECK-LABEL: @bs_all_operand64(
+; CHECK-NEXT:    [[TMP1:%.*]] = and i64 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    ret i64 [[TMP1]]
+;
+  %1 = tail call i64 @llvm.bswap.i64(i64 %a)
+  %2 = tail call i64 @llvm.bswap.i64(i64 %b)
+  %3 = and i64 %1, %2
+  %4 = tail call i64 @llvm.bswap.i64(i64 %3)
+  ret i64 %4
+}
+
+define i64 @bs_all_operand64_multiuse_both(i64 %a, i64 %b) #0 {
+; CHECK-LABEL: @bs_all_operand64_multiuse_both(
+; CHECK-NEXT:    [[TMP1:%.*]] = tail call i64 @llvm.bswap.i64(i64 [[A:%.*]])
+; CHECK-NEXT:    [[TMP2:%.*]] = tail call i64 @llvm.bswap.i64(i64 [[B:%.*]])
+; CHECK-NEXT:    [[TMP3:%.*]] = and i64 [[TMP1]], [[TMP2]]
+; CHECK-NEXT:    [[TMP4:%.*]] = tail call i64 @llvm.bswap.i64(i64 [[TMP3]])
+; CHECK-NEXT:    call void @use.i64(i64 [[TMP1]])
+; CHECK-NEXT:    call void @use.i64(i64 [[TMP2]])
+; CHECK-NEXT:    ret i64 [[TMP4]]
+;
+  %1 = tail call i64 @llvm.bswap.i64(i64 %a)
+  %2 = tail call i64 @llvm.bswap.i64(i64 %b)
+  %3 = and i64 %1, %2
+  %4 = tail call i64 @llvm.bswap.i64(i64 %3)
+
+  call void @use.i64(i64 %1)
+  call void @use.i64(i64 %2)
+  ret i64 %4
+}
+
 define i64 @bs_active_high8(i64 %0) {
 ; CHECK-LABEL: @bs_active_high8(
 ; CHECK-NEXT:    [[TMP2:%.*]] = and i64 [[TMP0:%.*]], 255
 ; CHECK-NEXT:    ret i64 [[TMP2]]
 ;
   %2 = shl i64 %0, 56
   %3 = call i64 @llvm.bswap.i64(i64 %2)
   ret i64 %3

 ; CHECK-NEXT:    ret i64 [[TMP3]]
 ;
   %2 = and i64 %0, 384  ; 0x0180
   %3 = call i64 @llvm.bswap.i64(i64 %2)
   ret i64 %3
 }
 
 
+declare i64 @use.i64(i64)
 declare i16 @llvm.bswap.i16(i16)
 declare i32 @llvm.bswap.i32(i32)
 declare i64 @llvm.bswap.i64(i64)
 declare <2 x i16> @llvm.bswap.v2i16(<2 x i16>)
 declare <2 x i32> @llvm.bswap.v2i32(<2 x i32>)
 declare <2 x i64> @llvm.bswap.v2i64(<2 x i64>)