This is an archive of the discontinued LLVM Phabricator instance.

[NewPM][LoopUnroll] Rename unroll* to loop-unroll*
ClosedPublic

Authored by aeubanks on Jun 25 2020, 11:49 AM.

Details

Summary

The legacy pass is called "loop-unroll", but in the new PM it's called "unroll".
Also applied to unroll-and-jam and unroll-full.

Fixes various check-llvm tests when NPM is turned on.

Diff Detail

Event Timeline

aeubanks created this revision.Jun 25 2020, 11:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 25 2020, 11:49 AM
dmgreen added inline comments.
llvm/lib/Passes/PassRegistry.def
274

It's called "unroll". So why would it not be called "unroll-and-jam"?
(I have no real opinion on one vs the other, but seems more consistent to use a name similar to the unroll pass)

How about change unroll to loop-unroll as well? In that case unroll and unroll-and-jam name can be more consistent, and their name can also be consistent between pass mangers.

aeubanks marked an inline comment as done.Jun 25 2020, 3:05 PM
aeubanks added inline comments.
llvm/lib/Passes/PassRegistry.def
274

I kept going back and forth on which one to use. Other related passes are named the same way, "loop-unroll-*" in legacy PM and "unroll-*" in NPM. I think having the "loop-" prefix is clearer, and also would require cleaning up fewer tests.
I can clean up each pass separately.

Whitney accepted this revision.Jun 25 2020, 3:21 PM
This revision is now accepted and ready to land.Jun 25 2020, 3:21 PM
aeubanks updated this revision to Diff 273532.Jun 25 2020, 3:23 PM

Handle all unroll passes

aeubanks marked an inline comment as done.Jun 25 2020, 3:24 PM
aeubanks added inline comments.
llvm/lib/Passes/PassRegistry.def
274

Never mind I just did it all here, can you take a look again?

aeubanks retitled this revision from [NewPM][LoopUnrollAndJam] Rename unroll-and-jam to loop-unroll-and-jam to [NewPM][LoopUnroll] Rename unroll* to loop-unroll*.Jun 25 2020, 3:25 PM
aeubanks edited the summary of this revision. (Show Details)
dmgreen accepted this revision.Jun 26 2020, 12:21 AM

Fair enough. You could say that the extra 5 character are superfluous, but if consistency between old and new pass managers is useful to you then this sounds OK to me.

This revision was automatically updated to reflect the committed changes.