This is an archive of the discontinued LLVM Phabricator instance.

[SystemZ] Improve handling of instructions which expand to several groups
ClosedPublic

Authored by jonpa on Aug 2 2018, 7:54 AM.

Details

Reviewers
uweigand
Summary

Some instructions expand to more than one decoder group.

This has been hitherto ignored, but is handled with this patch.

  • Changes in getNumDecoderSlots() is an NFC pre-patch. Since we changed NumMicroOps to reflect the number of decoder slots used, that value can be used directly here now. This could be committed by itself...
  • Cracked grouping SchedWrite removed from older subtargets (unused). Could also be committed beforehand.
  • Change name from GroupAlone[12] to Expanded[12] ?

Diff Detail

Event Timeline

jonpa created this revision.Aug 2 2018, 7:54 AM
uweigand added inline comments.Aug 2 2018, 10:31 AM
lib/Target/SystemZ/SystemZHazardRecognizer.cpp
157

Why would the number of groups affect the execution units here?

I though use of multiple execution units / use for multiple cycles was already covered via the resource lists (e.g. FXa2) ...

jonpa marked an inline comment as done.Aug 3 2018, 12:32 AM
jonpa added inline comments.
lib/Target/SystemZ/SystemZHazardRecognizer.cpp
157

At this point of completing a decoder group, *all* processor resource counters with a positive value are decremented. This is not related to any particular instruction, but rather to the advancement of some sort of "cycle" / "time".

So the change here means that in those cases where 2/3 groups are used, an extra decrement on all counters occur.

This would also make sense on a single instruction: Let's say SU uses LSU(2). If the LSU counter was 0, and SU groups alone with 1 group, the SU would first be emitted where LSU counter becomes 2. Then, the group would complete, and the counter become 1. This makes sense in the way that there generally (ideally) would be 1 LSU unit placed / available per decoder group. So the resulting '1' of the counter means +1 compared to the ideal...

If instead this SU would use 2 groups, the counter is instead decremented with 2, so it becomes 0 instead of one.

So, the resources list decide how each SU increments the counters, but the general time is what decrements them...

uweigand accepted this revision.Aug 3 2018, 2:58 AM

Patch LGTM. Thanks!

lib/Target/SystemZ/SystemZHazardRecognizer.cpp
154

That first if is now redundant, right?

157

Ah, I see. Makes sense.

This revision is now accepted and ready to land.Aug 3 2018, 2:58 AM
jonpa closed this revision.Aug 3 2018, 3:46 AM
jonpa marked 2 inline comments as done.

Committed as r338849.

lib/Target/SystemZ/SystemZHazardRecognizer.cpp
154

Yes, wasn't sure if I should remove it... Looking at it now again, maybe I should? Suppose the compiler (building llvm) should optimize this just as well if I removed it?