This is an archive of the discontinued LLVM Phabricator instance.

[X86][XOP] VFRCZ* instructions should be in their own sched class
AbandonedPublic

Authored by lebedev.ri on Sep 1 2018, 2:58 AM.

Details

Summary

VFRCZ* are AMD XOP instructions.
That instruction set is only supported by bdver[1-4] cpu models.

These instructions are unrelated to the instructions grouped by the WriteFRnd sched class.
On bdver[1-3], they have latency of 10, as per agner's tables,
and as per my llvm-exegesis measurements for bdver2.
Which is different from what WriteFRnd specifies (5).

Now, the test coverage is saddening. We currently don't have any
dedicated sched models for any of these 4 cpu's, so the test coverage decreases.
I'm not sure this can be improved, until the said sched model is ready for integration...

Diff Detail

Repository
rL LLVM

Event Timeline

lebedev.ri created this revision.Sep 1 2018, 2:58 AM

I think you'd be much better off just adding InstRW overrides to your prototype model, similar to how SSE4A instructions are handled in the btver2/znver1 models. Once we have actual models in trunk that support these instructions we can revisit this if you have a strong use case.

lib/Target/X86/X86SchedSandyBridge.td
15

This comment explains that SNB must support all models as its used as the generic model.

test/tools/llvm-mca/X86/Generic/resources-xop.s
2

You are missing the whole point of the generic model by doing this.

lebedev.ri added inline comments.Sep 3 2018, 4:14 AM
lib/Target/X86/X86SchedSandyBridge.td
15

Aha.
I suppose that also answers my unasked question as to why there is
two distinct ways to mark 'instruction' as unsupported - here for schedule class,
and ProcessorFeatures.

test/tools/llvm-mca/X86/Generic/resources-xop.s
2

Indeed i do.

Can this proceed if i fix this to not regress the default sched class?

Can this proceed if i fix this to not regress the default sched class?

I'd much prefer to wait until we have a scheduler model for a cpu that supports this.

Abandon this?

Abandon this?

The issue still is there, these are very different from the WriteFRnd.
https://github.com/llvm-mirror/llvm/blob/3a8afbd8a1a153d49969b13c1d8ba3858d860acd/lib/Target/X86/X86ScheduleBdVer2.td#L749-L779
Unless we prefer *lots* of InstRW, i should probably rebase this instead.

lebedev.ri abandoned this revision.Aug 12 2020, 2:26 PM