This is an archive of the discontinued LLVM Phabricator instance.

[TableGen] Rename ResourceCycles and StartAtCycle to clarify semantics
ClosedPublic

Authored by michaelmaitland on Aug 22 2023, 5:09 PM.

Details

Summary

D150312 added a TODO:

TODO: consider renaming the field StartAtCycle and Cycles to
AcquireAtCycle and ReleaseAtCycle respectively, to stress the
fact that resource allocation is now represented as an interval,
relatively to the issue cycle of the instruction.

This patch implements that TODO. This naming clarifies how to use these
fields in the scheduler. In addition it was confusing that StartAtCycle was
singular but Cycles was plural. This renaming fixes this inconsistency.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald Transcript
michaelmaitland requested review of this revision.Aug 22 2023, 5:09 PM
fpetrogalli accepted this revision.Aug 23 2023, 6:07 AM

Thank you for doing this @michaelmaitland.

Patch looks great. Just one minor thing. I believe there is a typo all across the patch: it should be Acquire, not Aquire.

Francesco

llvm/include/llvm/CodeGen/MachineScheduler.h
660
llvm/include/llvm/Target/TargetSchedule.td
257

This is very useful, thank you.

llvm/test/TableGen/AquireAtCycle.td
1

File name has typo too.

This revision is now accepted and ready to land.Aug 23 2023, 6:07 AM
michaelmaitland marked 2 inline comments as done.

It is spelled Acquire, not Aquire... (:embarrased:)

michaelmaitland marked an inline comment as done.Aug 23 2023, 6:23 AM
fpetrogalli accepted this revision.Aug 23 2023, 6:23 AM
This revision was landed with ongoing or failed builds.Aug 24 2023, 11:20 AM
This revision was automatically updated to reflect the committed changes.

I am failing build and will look into it. I built all targets locally and had no problem, but perhaps I missed something.

michaelmaitland reopened this revision.Aug 24 2023, 3:40 PM
This revision is now accepted and ready to land.Aug 24 2023, 3:40 PM

SystemZ Cycles -> ReleaseAtCycle

This revision was landed with ongoing or failed builds.Aug 24 2023, 7:21 PM
This revision was automatically updated to reflect the committed changes.

Is there any example show how AcquireAtCycles can be used to model scheduling? It's all 0s by default? I don't really understand how it works. :-)

Is there any example show how AcquireAtCycles can be used to model scheduling? It's all 0s by default? I don't really understand how it works. :-)

Working on improving the SiFive7 to use it. Will add you as reviewer once posted.

Is there any example show how AcquireAtCycles can be used to model scheduling? It's all 0s by default? I don't really understand how it works. :-)

You can watch the talk that describes what StartAtCycle (which became AcquireAtCycle) is for: https://www.youtube.com/watch?v=XWBVLcdzmFg

wangpc added a comment.Sep 4 2023, 4:54 AM

Is there any example show how AcquireAtCycles can be used to model scheduling? It's all 0s by default? I don't really understand how it works. :-)

You can watch the talk that describes what StartAtCycle (which became AcquireAtCycle) is for: https://www.youtube.com/watch?v=XWBVLcdzmFg

Oh thanks! That's really helpful! I understand it now, thanks for your admirable works! :-)