This is an archive of the discontinued LLVM Phabricator instance.

[NFC][AArch64][CodeGen] Use ArrayRef in TargetLowering functions
AbandonedPublic

Authored by sunshaoce on Apr 27 2022, 1:21 AM.

Details

Summary

Based on D123467.

Diff Detail

Event Timeline

sunshaoce created this revision.Apr 27 2022, 1:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2022, 1:21 AM
sunshaoce requested review of this revision.Apr 27 2022, 1:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2022, 1:21 AM

Can you provide a little detail as to the value of this change. I find it common to grep for setOperationAction(ISD::blah to identify the legalisation strategy. After this change that becomes impossible. I'm not saying I'm against this change but it would be nice to know what we get back in return for loosing this flexibility.

IMO, at least some of these modifications can be employed.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
435–436

The main purpose of this implementation is to reduce code duplication.

763–768

Also, I've noticed that similar measures have been taken in many places.

david-arm added inline comments.Apr 28 2022, 5:36 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
378

I can definitely some benefit here in terms of removing duplication, but in other places (I've left comments in a couple of examples) it's not obvious what the benefit is.

881

This change looks more cosmetic than functional - we're not really reducing lines of code here.

968

Again, not sure of the benefit of changes like this - we've not removed duplication, but it has made it harder to grep for operation actions.

sunshaoce abandoned this revision.May 23 2022, 9:07 PM