Page MenuHomePhabricator

[AArch64][SVE] Add patterns for integer mla/mls.
ClosedPublic

Authored by efriedma on Aug 10 2020, 12:31 PM.

Details

Summary

We probably want to introduce pseudo-instructions at some point, like we have for binary operations, but this seems okay for now.

One thing I'm not sure about is whether we should be doing this as a DAGCombine instead of directly pattern-matching it. I don't see any big downside to doing it this way, but maybe there's some important heuristic I'm not thinking of.

Diff Detail

Event Timeline

efriedma created this revision.Aug 10 2020, 12:31 PM
efriedma requested review of this revision.Aug 10 2020, 12:31 PM

I guess it depends on whether MLA hides the cost of the MUL for all implementations. We do seem to do this transformation is several places (DAG, ISEL and MachineCombiner) presumably there is a good reason for each placement.

Personally I prefer the DAG route because it forces the creation of a _PRED node, which we'll need sooner or later when the pseudo nodes exist[1]. It will also allow us to convert mla intrinsics for the "all active" case plus fixed length code generation can use it.

That all said this is a journey not a destination so I've no objections to the current patch if that's how far you want to take it today.

[1] I'm pretty sure all the mechanics are in place for MLA like pseudo nodes (at least for _PRED, less so for _MERGE_ZERO).

I guess it depends on whether MLA hides the cost of the MUL for all implementations.

Oh, hmm, I see what you mean. I'll restrict the pattern to match multiplies with one use.

Personally I prefer the DAG route because it forces the creation of a _PRED node, which we'll need sooner or later when the pseudo nodes exist[1]. It will also allow us to convert mla intrinsics for the "all active" case plus fixed length code generation can use it.

I don't see why fixed-length code generation needs this; you can use similar patterns for ADD_PRED.

efriedma updated this revision to Diff 284844.Aug 11 2020, 12:08 PM

Add one-use restriction.

paulwalker-arm accepted this revision.Aug 11 2020, 3:57 PM

Fair enough I just figured it would be good to not have multiple patterns for the same target instruction.

This revision is now accepted and ready to land.Aug 11 2020, 3:57 PM
This revision was automatically updated to reflect the committed changes.