This is an archive of the discontinued LLVM Phabricator instance.

[ARC] Add codegen for llvm.ctlz intrinsic for the ARC backend
AcceptedPublic

Authored by thomasjohns on Aug 5 2021, 4:21 PM.

Details

Reviewers
marksl
Summary

Add a CTLZ pseudo instruction to tablegen and the ability to expand this pseudo instruction instruction to ARC assembly code.

Diff Detail

Event Timeline

thomasjohns created this revision.Aug 5 2021, 4:21 PM
thomasjohns requested review of this revision.Aug 5 2021, 4:21 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 5 2021, 4:21 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
thomasjohns added inline comments.Aug 5 2021, 4:48 PM
llvm/lib/Target/ARC/ARCInstrInfo.td
322

I just realized I made a bad merge here and dropped the }. Fixing it now.

thomasjohns added inline comments.Aug 5 2021, 4:51 PM
llvm/lib/Target/ARC/ARCInstrInfo.td
322

I'll also move this pseudo instruction to be next to the other pseudo instructions

Fix missing } after bad merge. Move CTLZ next to other pseudo instructions.

llvm clang-tidy rule: prefer Register over unsinged int.

marksl added inline comments.Aug 6 2021, 8:59 AM
llvm/lib/Target/ARC/ARCExpandPseudos.cpp
86

I know you're following ExpandStore above in using "SI" here, but let's switch to "MI" as that is the most common practice. I think he used "SI" because it was a store instruction. All your readers will recognize MI as a MachineInstr, but they will have no such association to SI.

93

This is not right. The final instruction of the sequence must write to DestReg.

Thanks for the feedback, Mark.

This change prefers MI over SI naming, updates instruction writing, and makes the test case more precise (the generated code was improved and no longer had an unneeded mov %r1, %r0.

marksl accepted this revision.Aug 6 2021, 11:36 AM

Looks good

This revision is now accepted and ready to land.Aug 6 2021, 11:36 AM