Page MenuHomePhabricator

[AArch64] Add support for lowering the bitreverse intrinsic to the rbit instruction.
ClosedPublic

Authored by mcrosier on Jan 5 2017, 1:51 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

mcrosier updated this revision to Diff 83302.Jan 5 2017, 1:51 PM
mcrosier retitled this revision from to [AArch64] Add support for lowering the bitreverse intrinsic to the rbit instruction..
mcrosier updated this object.
mcrosier added subscribers: RKSimon, llvm-commits.
gberry added inline comments.Jan 5 2017, 2:16 PM
lib/Target/AArch64/AArch64InstrInfo.td
958 ↗(On Diff #83302)

I think you can get rid of these patterns and instead just change the defm RBIT above to pass the bitreverse fragment as the 3rd argument (as is done for CLZ).

RKSimon added inline comments.Jan 6 2017, 12:56 AM
lib/Target/AArch64/AArch64InstrInfo.td
957 ↗(On Diff #83302)

Are these target intrinsics still necessary?

mcrosier updated this revision to Diff 83366.Jan 6 2017, 7:46 AM

Address Simon and Geoff's feedback.

Add LowerINTRINSIC_WO_CHAIN or AutoUpgrade support to handle int_aarch64_rbit with bitreverse? See ARMTargetLowering::LowerINTRINSIC_WO_CHAIN

mcrosier marked an inline comment as done.
mcrosier added inline comments.
lib/Target/AArch64/AArch64InstrInfo.td
957 ↗(On Diff #83302)

Good point, Simon. I believe this patch and D28400 together can allow us to remove the target-specific intrinsic.

mcrosier updated this revision to Diff 83370.Jan 6 2017, 8:31 AM

Add support for lowering aarch64_rbit to BITREVERSE DAG node, per Simon's comment. Thanks, Simon.

RKSimon added inline comments.Jan 8 2017, 4:13 AM
include/llvm/IR/IntrinsicsAArch64.td
45 ↗(On Diff #83370)

You can remove this right now if you add support in AutoUpgrade.cpp

mcrosier updated this revision to Diff 83633.Jan 9 2017, 8:50 AM

-Use autoupgrade to allow removal of aarch64 rbit intrinsic, per Simon's suggestion.

mcrosier updated this revision to Diff 83634.Jan 9 2017, 8:54 AM

Update a comment now that we're auto-upgrading.

RKSimon edited edge metadata.Jan 9 2017, 8:58 AM

No further comments from me.

No further comments from me.

Thanks, Simon.

mcrosier updated this object.Jan 10 2017, 7:05 AM
mcrosier edited edge metadata.

Ping (AArch64 contributors). This patch is in great shape thanks to Simon, but I assume he's deferring the official LGTM to a more frequent AArch64 contributor.

sbaranga accepted this revision.Jan 10 2017, 8:35 AM
sbaranga added a reviewer: sbaranga.
sbaranga added a subscriber: sbaranga.

LGTM!

This revision is now accepted and ready to land.Jan 10 2017, 8:35 AM

LGTM!

Thanks, Silviu! Would you mind taking a quick look at the clang side patch: D28400? Very straight forward.

This revision was automatically updated to reflect the committed changes.