This is an archive of the discontinued LLVM Phabricator instance.

[X86] Update the haswell and broadwell scheduler information for gather instructions
ClosedPublic

Authored by craig.topper on Feb 1 2020, 10:17 PM.

Details

Summary

Broadwell was missing half the gather instructions. Both models
had some mixups in the resource costs and number of uops.

I've updated here based on what I think the original IACA source
says with some cross checking against the microcode.

I'm not sure about latency as the IACA source I have doesn't have
that information. So I'm using the latency from uops.info.

I plan to update Skylake models as well, but I'll do that in a
separate patch.

Diff Detail

Event Timeline

craig.topper created this revision.Feb 1 2020, 10:17 PM
Herald added a project: Restricted Project. · View Herald Transcript

PR35676 discusses the missing Haswell/Broadwell gather schedules
PR35675 discusses poor Skylake (Client/Server) gather schedules

The change looks pretty mechanical, and I trust that the new numbers are correct.

I noticed that gather instructions in tablegen are all associated with Sched<[WriteLoad]> (see multiclass avx2_gather in X86InstrSSE.td).
A quick grep of GATHER in directory Target/X86 shows how most (if not all?) avx2 processor models currently introduce InstRW definitions to override the scheduling class of gather instructions.
Basically, WriteLoad is clearly not a good choice for gather instructions. Presumably that is the reason why there is a FIXME comment on avx2_gather (in X86InstrSSE.td) that writes "Improve sheduling of gather instructions.".

Here is the idea: why don't we just add new writes for gather instructions (ideally a new write per each type of gather)?. I think that we shall consider this alternative (maybe in a follow up patch? I don't particularly mind..). Not sure if other people agree with this.

Here is the idea: why don't we just add new writes for gather instructions (ideally a new write per each type of gather)?. I think that we shall consider this alternative (maybe in a follow up patch? I don't particularly mind..). Not sure if other people agree with this.

Adding gather (and broadcast) schedule classes is on my backlog - as long as the InstRW entries are correct, I'll replace them when I get aroung to doing it. So, staying with InstRW for this patch is fine by me.

andreadb accepted this revision.Feb 3 2020, 6:20 AM

Here is the idea: why don't we just add new writes for gather instructions (ideally a new write per each type of gather)?. I think that we shall consider this alternative (maybe in a follow up patch? I don't particularly mind..). Not sure if other people agree with this.

Adding gather (and broadcast) schedule classes is on my backlog - as long as the InstRW entries are correct, I'll replace them when I get aroung to doing it. So, staying with InstRW for this patch is fine by me.

Sounds like a good plan to me.

I don't have other comments. Assuming that numbers are correct, the patch looks good to me.

This revision is now accepted and ready to land.Feb 3 2020, 6:20 AM

Fix mistake in uop count and resource cycles for VGATHERQPSrm and VPGATHERQDrm

This revision was automatically updated to reflect the committed changes.