[PATCH] D54648: [TableGen] Emit more variant transitions

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

Hi Evandro,

Sorry for the very late reply.
I am back from holidays. I plan to review this patch and your other patch (https://reviews.llvm.org/D54777).

More in general: I am very happy to see that there is interest in llvm-mca from people working on aarch64. So, I will do my best to help you with reviews and suggesting alternative approaches.

> llvm-mca relies on the predicates to be based on MCSchedPredicate in order to resolve the scheduling for variant instructions. Otherwise, it aborts the building of the instruction model early.
>  However, the scheduling model emitter in TableGen gives up too soon, unless all processors use only such predicates.

This is done intentionally.

Transitions are expanded into an `if-then-else` sequence, and scheduling predicates are predicate expressions used to check if a branch (of the if-then-else construct) is taken or not.
Predicates have to be processed strictly in sequence, and the order matters.

> In order to allow more processors to be used with llvm-mca, this patch emits scheduling transitions if any processor uses these predicates. The transition emitted for the processors using legacy predicates is the one specified with NoSchedPred, which is based on MCSchedPredicate.

I don't think this is the right approach. As I mentioned before, predicates must be processed in sequence.

We cannot just emit "some" transitions and pretend that everything works fine; the resolved scheduling class would be potentially incorrect. We cannot assume that NoSchedPred is a good guess (i.e. default) for most scheduling write variants...  NoSchedPred is just the last predicate of the sequence; we cannot make assumptions on how often it is used to resolve write variants. Using it as a default might be okay for your target processor in some cases. But it would lead to inaccurate results on other target processors.

In all honesty. If you really care about supporting llvm-mca for aarch64, then the right approach is to port existing scheduling predicates to the new MCSchedPredicate framework.
This patch sound more like a hack to get llvm-mca working for some Aarch64 instructions. I personally don't like it.

On the other hand, your https://reviews.llvm.org/D54777 seems to go the right direction. I plan to review it next.

-Andrea


https://reviews.llvm.org/D54648