Page MenuHomePhabricator

[RISCV] Use a macro to simplify getTargetNodeName
ClosedPublic

Authored by frasercrmck on Nov 13 2020, 4:34 AM.

Details

Summary

Similar to the X86 and AMDGPU targets, this uses a macro to cut down on
repetitive and error-prone code when converting RISCVISD node names to
strings in getTargetNodeName.

Diff Detail

Event Timeline

frasercrmck created this revision.Nov 13 2020, 4:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 13 2020, 4:34 AM
frasercrmck requested review of this revision.Nov 13 2020, 4:34 AM

The formatting was done by clang-format and it does look a little "off". We can go against it if people like.

If anyone's concerned about merge conflicts with ongoing work, just shout. This isn't the most important thing to get in ASAP, after all.

asb accepted this revision.Nov 13 2020, 6:19 AM

I think moving NODE_NAME_CASE(foo) to the left so it's aligned where the case statement would be (same as the equivalent in X86ISelLowering and AMDGPUISelLowering) would be slightly better. I wouldn't be opposed to using // clang-format off to stop clang-format from reformatting (though it seems more common in LLVM right now to just ignore clang-format's preference).

But basically this looks good to me, and I leave it to your judgement if you want to further mess with the whitespace as suggested above. Thanks Fraser!

This revision is now accepted and ready to land.Nov 13 2020, 6:19 AM
In D91414#2393793, @asb wrote:

I think moving NODE_NAME_CASE(foo) to the left so it's aligned where the case statement would be (same as the equivalent in X86ISelLowering and AMDGPUISelLowering) would be slightly better. I wouldn't be opposed to using // clang-format off to stop clang-format from reformatting (though it seems more common in LLVM right now to just ignore clang-format's preference).

But basically this looks good to me, and I leave it to your judgement if you want to further mess with the whitespace as suggested above. Thanks Fraser!

Yep okay I'll align the cases left and go with clang-format off/on even though it's rarely used in the codebase. That should help to avoid having people use clang-format and wonder what's going on or manually having to undo bits. It's meant to make developers' lives easier, after all :)

This revision was landed with ongoing or failed builds.Nov 16 2020, 1:40 AM
This revision was automatically updated to reflect the committed changes.