This is an archive of the discontinued LLVM Phabricator instance.

[TableGen] Emit more variant transitions
ClosedPublic

Authored by evandro on Nov 16 2018, 1:34 PM.

Details

Summary

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.

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.

Diff Detail

Event Timeline

evandro created this revision.Nov 16 2018, 1:34 PM
evandro updated this revision to Diff 174684.Nov 19 2018, 2:39 PM
evandro edited the summary of this revision. (Show Details)

Exclude test that does not apply anymore.

evandro added a subscriber: mattd.Nov 19 2018, 2:41 PM

Hi Evandro,

Sorry for the very late reply.
I am back from holidays. I plan to review this patch and your other patch (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 D54777 seems to go the right direction. I plan to review it next.

-Andrea

The issue that I'm trying to avoid is that it's not enough for me to add predicates based on MCSchedPredicate for Exynos processors is other processors don't. Then, if an instruction that I model by using a variant schedule is also modeled by another processor, TableGen will emit no solution at all for the instruction. This patch, which I recognize is just an attempt, aims at allowing the proper solution for a processor using such predicates, while indeed resulting in a clumsy solution the scheduling of the same instruction for other processors.

The issue is that it's virtually impossible at the moment to model AArch64 without running on llvm-mca giving right up. I was thinking that instead of giving up, llvm-mca should resort to a reasobale default and highlight it in its result. I proposed NoSchedPred as this default, but, though we can discuss what the default should be, I think that no default does not make sense as is.

ormris added a subscriber: ormris.Nov 21 2018, 11:38 AM
andreadb accepted this revision.Nov 21 2018, 12:28 PM

The issue that I'm trying to avoid is that it's not enough for me to add predicates based on MCSchedPredicate for Exynos processors is other processors don't. Then, if an instruction that I model by using a variant schedule is also modeled by another processor, TableGen will emit no solution at all for the instruction. This patch, which I recognize is just an attempt, aims at allowing the proper solution for a processor using such predicates, while indeed resulting in a clumsy solution the scheduling of the same instruction for other processors.

The issue is that it's virtually impossible at the moment to model AArch64 without running on llvm-mca giving right up. I was thinking that instead of giving up, llvm-mca should resort to a reasobale default and highlight it in its result. I proposed NoSchedPred as this default, but, though we can discuss what the default should be, I think that no default does not make sense as is.

Right. I see what you mean.

I was under the impression that transitions would have been correctly expanded at least for your Exynos models.
However, it seems like the lack of MCSchedPredicates in other processor models is somehow causing problems.. If that's the case, then I think your patch makes sense.
Although it is not idea, at least, it unblocks your work.

Could you add a brief comment (a FIXME ) in the code, and the XFAIL test to explain what is happening (and why we are less strict with the check)?
We may want to revisit this in future. But for now, your patch is a good starting point.

Thanks!
-Andrea

This revision is now accepted and ready to land.Nov 21 2018, 12:28 PM
This revision was automatically updated to reflect the committed changes.