This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Optimize bitwise and with constant for the Zbs extension
ClosedPublic

Authored by benshi001 on Jun 4 2021, 10:45 PM.

Details

Summary

This patch optimizes (and r i) to
(BCLRI (BCLRI r, i0), i1) in which i = ~((1<<i0) | (1<<i1)).
or
(BCLRI (ANDI r, i0), i1) in which i = i0 & ~(1<<i1).

Diff Detail

Event Timeline

benshi001 created this revision.Jun 4 2021, 10:45 PM
benshi001 requested review of this revision.Jun 4 2021, 10:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 4 2021, 10:45 PM
craig.topper added inline comments.Jun 5 2021, 12:38 PM
llvm/lib/Target/RISCV/RISCVInstrInfoB.td
140

What if the type is i32 and bit 31 is one of the clear bits. This sign extend won't handle that case.

benshi001 updated this revision to Diff 350087.Jun 5 2021, 10:38 PM
benshi001 marked an inline comment as done.Jun 5 2021, 10:41 PM
benshi001 added inline comments.
llvm/lib/Target/RISCV/RISCVInstrInfoB.td
140

I have handled the special case you concerned, and two tests are added along, which show improvements of the special case.

It is not needed to pre-commit the tests, since the difference among the pattern

RV32I, RV32IB and RV32IBS shows this improvement.

benshi001 marked an inline comment as done.Jun 6 2021, 7:35 AM

Are you planning on extending this to use bclriw on RV64? (Would that clash with the use of tablegen? Personally, I tend to find C++ custom lowering of longer stuff easier to read and more cohesive, not sure how others feel)

llvm/lib/Target/RISCV/RISCVInstrInfoB.td
140

Drop the explicit cast to uint64_t?

141–143

I found this a bit confusing when I first read it. I suppose an alternative would be to replace the three lines with something like return countPopulation(I) == XLen - 2?

benshi001 updated this revision to Diff 350289.Jun 7 2021, 7:48 AM
benshi001 marked 2 inline comments as done.

Are you planning on extending this to use bclriw on RV64? (Would that clash with the use of tablegen? Personally, I tend to find C++ custom lowering of longer stuff easier to read and more cohesive, not sure how others feel)

I have not consider BSETIW/BINVIW/BCLRIW, maybe I will do it in next patch. And this patch will just focuses on BCLRI.

I do not like C++ custom lowering for that, since it needs even more lines than current form.

benshi001 updated this revision to Diff 350294.Jun 7 2021, 7:57 AM

Are you planning on extending this to use bclriw on RV64? (Would that clash with the use of tablegen? Personally, I tend to find C++ custom lowering of longer stuff easier to read and more cohesive, not sure how others feel)

I have not consider BSETIW/BINVIW/BCLRIW, maybe I will do it in next patch. And this patch will just focuses on BCLRI.

I do not like C++ custom lowering for that, since it needs even more lines than current form.

Weren’t the W Zbs instructions removed from the 0.94 spec?

This revision is now accepted and ready to land.Jun 7 2021, 10:29 AM
This revision was landed with ongoing or failed builds.Jun 7 2021, 4:26 PM
This revision was automatically updated to reflect the committed changes.