This is an archive of the discontinued LLVM Phabricator instance.

Differential D79267

[RISCV][NFC] Add Test for (select (or B1, B2), X, Y)
ClosedPublic

Authored by lenary on May 1 2020, 2:33 PM.

Details

Reviewers
luismarques
Commits
rGc44266dc4816: [RISCV][NFC] Add Test for (select (or B1, B2), X, Y)
Summary

As shown, LLVM is keen to avoid logic and introduce selects (in DAGCombiner, and
other places). This leads to control flow on RISC-V which we should attempt to
avoid.

Diff Detail

Unit TestsFailed

TimeTest
10,180 mslinux > libomp.env::Unknown Unit Message ("")
1,900 mslinux > libomp.worksharing/for::Unknown Unit Message ("")

Event Timeline

lenary created this revision.May 1 2020, 2:33 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 1 2020, 2:33 PM
Herald added subscribers: llvm-commits, evandro, apazos and 25 others. · View Herald Transcript
lenary added a child revision: D79268: [RISCV] Implement Hooks to avoid chaining SELECT.May 1 2020, 2:51 PM
luismarques added a comment.May 1 2020, 3:56 PM

LGTM.

llvm/test/CodeGen/RISCV/select-or.ll
8–9

Nit: if the codegen improvement isn't expected to be merged quickly then consider writing something more generic like "illustrates whether we use conditional jumps or additional logic", since for now it might be a bit confusing that the test does generate several branches.

luismarques accepted this revision.May 1 2020, 3:57 PM
This revision is now accepted and ready to land.May 1 2020, 3:57 PM
Harbormaster completed remote builds in B55495: Diff 261550.May 1 2020, 7:16 PM
lenary updated this revision to Diff 261643.May 2 2020, 7:22 AM

Update comment in test to more generally talk about what the test is looking
for, rather than the specific hook in the optimisation.

Harbormaster completed remote builds in B55548: Diff 261643.May 2 2020, 8:27 AM
lenary planned changes to this revision.May 5 2020, 5:08 AM

I might actually add a few other tests, given i know (select (and A, B), C, D) is also a pattern that appears places.

lenary updated this revision to Diff 274034.Jun 29 2020, 3:21 AM
  • Added (select (and A, B), X, Y) test.
This revision is now accepted and ready to land.Jun 29 2020, 3:21 AM
Herald added a subscriber: steven.zhang. · View Herald TranscriptJun 29 2020, 3:21 AM
Harbormaster failed remote builds in B62120: Diff 274034!Jun 29 2020, 4:17 AM
lenary updated this revision to Diff 274530.Jun 30 2020, 10:26 AM
  • Expand select(or/and) tests to include if(or/and) cases
lenary mentioned this in D79268: [RISCV] Implement Hooks to avoid chaining SELECT.Jun 30 2020, 10:35 AM
lenary updated this revision to Diff 274537.Jun 30 2020, 10:42 AM
  • Simplify and test to ensure branchy code is generated.
Harbormaster failed remote builds in B62363: Diff 274537!Jun 30 2020, 11:56 AM
Harbormaster failed remote builds in B62357: Diff 274530!
Closed by commit rGc44266dc4816: [RISCV][NFC] Add Test for (select (or B1, B2), X, Y) (authored by lenary). · Explain WhyJul 1 2020, 4:17 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
llvm/
test/
CodeGen/
RISCV/
88 lines
102 lines

Diff 274530

llvm/test/CodeGen/RISCV/select-and.ll

  • This file was added.
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc -mtriple=riscv32 -verify-machineinstrs < %s \
; RUN: | FileCheck -check-prefix=RV32I %s
; RUN: llc -mtriple=riscv64 -verify-machineinstrs < %s \
; RUN: | FileCheck -check-prefix=RV64I %s
;; There are a few different ways to lower (select (and A, B), X, Y). This test
;; ensures that we do so with as few branches as possible.
define signext i32 @select_of_and(i32 signext %a, i32 signext %b, i32 signext %c, i32 signext %d) nounwind {
; RV32I-LABEL: select_of_and:
; RV32I: # %bb.0:
; RV32I-NEXT: or a1, a0, a1
; RV32I-NEXT: mv a0, a2
; RV32I-NEXT: beqz a1, .LBB0_2
; RV32I-NEXT: # %bb.1:
; RV32I-NEXT: mv a0, a3
; RV32I-NEXT: .LBB0_2:
; RV32I-NEXT: ret
;
; RV64I-LABEL: select_of_and:
; RV64I: # %bb.0:
; RV64I-NEXT: or a1, a0, a1
; RV64I-NEXT: mv a0, a2
; RV64I-NEXT: beqz a1, .LBB0_2
; RV64I-NEXT: # %bb.1:
; RV64I-NEXT: mv a0, a3
; RV64I-NEXT: .LBB0_2:
; RV64I-NEXT: ret
%1 = icmp eq i32 %a, 0
%2 = icmp eq i32 %b, 0
%3 = and i1 %1, %2
%4 = select i1 %3, i32 %c, i32 %d
ret i32 %4
}
declare signext i32 @both() nounwind
declare signext i32 @neither() nounwind
define signext i32 @if_of_and(i1 zeroext %a, i1 zeroext %b) nounwind {
; RV32I-LABEL: if_of_and:
; RV32I: # %bb.0:
; RV32I-NEXT: addi sp, sp, -16
; RV32I-NEXT: sw ra, 12(sp)
; RV32I-NEXT: beqz a0, .LBB1_3
; RV32I-NEXT: # %bb.1:
; RV32I-NEXT: beqz a1, .LBB1_3
; RV32I-NEXT: # %bb.2: # %if.then
; RV32I-NEXT: call both
; RV32I-NEXT: j .LBB1_4
; RV32I-NEXT: .LBB1_3: # %if.else
; RV32I-NEXT: call neither
; RV32I-NEXT: .LBB1_4: # %if.end
; RV32I-NEXT: lw ra, 12(sp)
; RV32I-NEXT: addi sp, sp, 16
; RV32I-NEXT: ret
;
; RV64I-LABEL: if_of_and:
; RV64I: # %bb.0:
; RV64I-NEXT: addi sp, sp, -16
; RV64I-NEXT: sd ra, 8(sp)
; RV64I-NEXT: beqz a0, .LBB1_3
; RV64I-NEXT: # %bb.1:
; RV64I-NEXT: beqz a1, .LBB1_3
; RV64I-NEXT: # %bb.2: # %if.then
; RV64I-NEXT: call both
; RV64I-NEXT: j .LBB1_4
; RV64I-NEXT: .LBB1_3: # %if.else
; RV64I-NEXT: call neither
; RV64I-NEXT: .LBB1_4: # %if.end
; RV64I-NEXT: ld ra, 8(sp)
; RV64I-NEXT: addi sp, sp, 16
; RV64I-NEXT: ret
%1 = and i1 %a, %b
br i1 %1, label %if.then, label %if.else
if.then:
%2 = tail call i32 @both()
br label %if.end
if.else:
%3 = tail call i32 @neither()
br label %if.end
if.end:
%4 = phi i32 [%2, %if.then], [%3, %if.else]
ret i32 %4
}

llvm/test/CodeGen/RISCV/select-or.ll

  • This file was added.
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc -mtriple=riscv32 -verify-machineinstrs < %s \
; RUN: | FileCheck -check-prefix=RV32I %s
; RUN: llc -mtriple=riscv64 -verify-machineinstrs < %s \
; RUN: | FileCheck -check-prefix=RV64I %s
;; There are a few different ways to lower (select (or A, B), X, Y). This test
;; ensures that we do so with as few branches as possible.
luismarquesUnsubmitted
Not Done
ReplyInline Actions

Nit: if the codegen improvement isn't expected to be merged quickly then consider writing something more generic like "illustrates whether we use conditional jumps or additional logic", since for now it might be a bit confusing that the test does generate several branches.

luismarques: Nit: if the codegen improvement isn't expected to be merged quickly then consider writing…
define signext i32 @select_of_or(i32 zeroext %a, i32 zeroext %b, i32 signext %c, i32 signext %d) nounwind {
; RV32I-LABEL: select_of_or:
; RV32I: # %bb.0:
; RV32I-NEXT: mv a4, a2
; RV32I-NEXT: bnez a1, .LBB0_3
; RV32I-NEXT: # %bb.1:
; RV32I-NEXT: bnez a0, .LBB0_4
; RV32I-NEXT: .LBB0_2:
; RV32I-NEXT: mv a0, a2
; RV32I-NEXT: ret
; RV32I-NEXT: .LBB0_3:
; RV32I-NEXT: mv a4, a3
; RV32I-NEXT: beqz a0, .LBB0_2
; RV32I-NEXT: .LBB0_4:
; RV32I-NEXT: mv a2, a4
; RV32I-NEXT: mv a0, a2
; RV32I-NEXT: ret
;
; RV64I-LABEL: select_of_or:
; RV64I: # %bb.0:
; RV64I-NEXT: mv a4, a2
; RV64I-NEXT: bnez a1, .LBB0_3
; RV64I-NEXT: # %bb.1:
; RV64I-NEXT: bnez a0, .LBB0_4
; RV64I-NEXT: .LBB0_2:
; RV64I-NEXT: mv a0, a2
; RV64I-NEXT: ret
; RV64I-NEXT: .LBB0_3:
; RV64I-NEXT: mv a4, a3
; RV64I-NEXT: beqz a0, .LBB0_2
; RV64I-NEXT: .LBB0_4:
; RV64I-NEXT: mv a2, a4
; RV64I-NEXT: mv a0, a2
; RV64I-NEXT: ret
%1 = icmp eq i32 %a, 0
%2 = icmp eq i32 %b, 0
%3 = or i1 %1, %2
%4 = select i1 %3, i32 %c, i32 %d
ret i32 %4
}
declare signext i32 @either() nounwind
declare signext i32 @neither() nounwind
define signext i32 @if_of_or(i1 zeroext %a, i1 zeroext %b) nounwind {
; RV32I-LABEL: if_of_or:
; RV32I: # %bb.0:
; RV32I-NEXT: addi sp, sp, -16
; RV32I-NEXT: sw ra, 12(sp)
; RV32I-NEXT: bnez a0, .LBB1_3
; RV32I-NEXT: # %bb.1:
; RV32I-NEXT: bnez a1, .LBB1_3
; RV32I-NEXT: # %bb.2: # %if.else
; RV32I-NEXT: call neither
; RV32I-NEXT: j .LBB1_4
; RV32I-NEXT: .LBB1_3: # %if.then
; RV32I-NEXT: call either
; RV32I-NEXT: .LBB1_4: # %if.end
; RV32I-NEXT: lw ra, 12(sp)
; RV32I-NEXT: addi sp, sp, 16
; RV32I-NEXT: ret
;
; RV64I-LABEL: if_of_or:
; RV64I: # %bb.0:
; RV64I-NEXT: addi sp, sp, -16
; RV64I-NEXT: sd ra, 8(sp)
; RV64I-NEXT: bnez a0, .LBB1_3
; RV64I-NEXT: # %bb.1:
; RV64I-NEXT: bnez a1, .LBB1_3
; RV64I-NEXT: # %bb.2: # %if.else
; RV64I-NEXT: call neither
; RV64I-NEXT: j .LBB1_4
; RV64I-NEXT: .LBB1_3: # %if.then
; RV64I-NEXT: call either
; RV64I-NEXT: .LBB1_4: # %if.end
; RV64I-NEXT: ld ra, 8(sp)
; RV64I-NEXT: addi sp, sp, 16
; RV64I-NEXT: ret
%1 = or i1 %a, %b
br i1 %1, label %if.then, label %if.else
if.then:
%2 = tail call i32 @either()
br label %if.end
if.else:
%3 = tail call i32 @neither()
br label %if.end
if.end:
%4 = phi i32 [%2, %if.then], [%3, %if.else]
ret i32 %4
}