Page MenuHomePhabricator

[RFC][AArch64] Use the new MCSchedPredicate to rewrite a couple of predicates.
AbandonedPublic

Authored by andreadb on May 10 2018, 8:46 AM.

Details

Summary

This is related to "[RFC] MC support for variant scheduling classes" in LLVM-dev.

This is an example of how the new predicates can be used in the AArch64 backend.
This is not meant to be committed.

Diff Detail

Event Timeline

andreadb created this revision.May 10 2018, 8:46 AM
mattd added a subscriber: mattd.May 10 2018, 9:46 AM
javed.absar added inline comments.May 10 2018, 3:09 PM
lib/Target/AArch64/AArch64InstrInfoPredicates.td
32

Hi Andrea:
Maybe i am not getting this properly. But how is this approach easier than simply defining a function "IsGPRZero(const MachineInstr*)", as it is done presently.

gbedwell added inline comments.May 10 2018, 3:19 PM
lib/Target/AArch64/AArch64InstrInfoPredicates.td
32

The full RFC (including this example) can be seen at http://lists.llvm.org/pipermail/llvm-dev/2018-May/123181.html

Essentially, the new approach gives the ability to use the same semantic information for both MachineInstr and MCInst which is important for llvm-mca which operates entirely in the MCInst domain.

andreadb added inline comments.May 11 2018, 3:21 AM
lib/Target/AArch64/AArch64InstrInfoPredicates.td
32

Hi Javed,

it may not be easier than defining two functions (one for MachineInstr, another for MCInst). In some cases it may be less obvious to read.
I guess it depends on what use cases you (as a developer working on a scheduling model) want to support.
This new syntax mostly makes sense if you plan to expose knowledge about variant scheduling classes to a tool like llvm-mca.

If you don't, then I totally agree: there is no reason why you should use the new syntax.

My idea was: rather than forcing people to define multiple TII hooks (which are semantically the same), let's give to people a tool to automatically generate them.
It is meant to help the transition from SchedPredicate to MCPredicate in ARM/AArch64 models.

Not sure if this comment helps.
I hope it makes sense.

javed.absar added inline comments.May 11 2018, 5:28 AM
lib/Target/AArch64/AArch64InstrInfoPredicates.td
32

Thanks Andrea.
I see the point now. BTW, thanks a lot for this work.

andreadb added inline comments.May 11 2018, 6:08 AM
lib/Target/AArch64/AArch64InstrInfoPredicates.td
32

No problem.

I have also added a comment on the RFC thread suggesting an alternative approach (with an example).

Cheers,
Andrea

andreadb abandoned this revision.Jun 18 2018, 7:23 AM

Abandoning this patch as there was no concrete plan to push this change Upstrea. It was mainly to help the review process for the RFC patches.