Page MenuHomePhabricator

Add loop pragma for Loop Distribution
ClosedPublic

Authored by anemet on Apr 21 2016, 10:24 PM.

Details

Summary

This is similar to other loop pragmas like 'vectorize'. Currently it
only has state values: distribute(enable) and distribute(disable). When
one of these is specified the corresponding loop metadata is generated:

!{!"llvm.loop.distribute.enable", i1 true/false}

As a result, loop distribution will be attempted on the loop even if
Loop Distribution in not enabled globally. Analogously, with 'disable'
distribution can be turned off for an individual loop even when the pass
is otherwise enabled.

There are some slight differences compared to the existing loop pragmas.

  1. There is no 'assume_safety' variant which makes its handling slightly

different from 'vectorize'/'interleave'.

  1. Unlike the existing loop pragmas, it does not have a corresponding

numeric pragma like 'vectorize' -> 'vectorize_width'. So for the
consistency checks in CheckForIncompatibleAttributes we don't need to
check it against other pragmas. We just need to check for duplicates of
the same pragma.

Diff Detail

Repository
rL LLVM

Event Timeline

anemet updated this revision to Diff 54612.Apr 21 2016, 10:24 PM
anemet retitled this revision from to Add loop pragma for Loop Distribution.
anemet updated this object.
anemet added reviewers: aaron.ballman, rsmith.
anemet added subscribers: hfinkel, cfe-commits.
aaron.ballman added inline comments.Apr 28 2016, 5:58 AM
docs/LanguageExtensions.rst
2161 ↗(On Diff #54612)

It would be nice to use a more compelling example than an empty for loop. Also, it would be helpful if the user understood why they wouldn't use this pragma for every loop (or why it doesn't happen by default).

anemet added inline comments.Apr 28 2016, 9:32 AM
docs/LanguageExtensions.rst
2161 ↗(On Diff #54612)

Sure, I just followed the vectorizer example but you're right this needs more context. Let me update the patch.

Thanks for the review!

anemet updated this revision to Diff 55439.Apr 28 2016, 10:39 AM

Added more context to the documentation per Aaron's comments.

anemet updated this revision to Diff 55445.Apr 28 2016, 10:52 AM

A little tweak to the example to make it clear that 'i' is the induction
variable

aaron.ballman accepted this revision.Apr 28 2016, 12:41 PM
aaron.ballman edited edge metadata.

LGTM, but you should wait in case @rsmith has feedback.

This revision is now accepted and ready to land.Apr 28 2016, 12:41 PM
anemet added a comment.May 2 2016, 9:37 AM

@rsmith, hi! Do you have any comments on this or you're OK with this per Aaron's LGTM?

Thanks,
Adam

anemet added a comment.May 9 2016, 3:43 PM

Ping.

(I am getting a bit concerned that because this was already marked accepted, it does not show up @rsmith' active queue...)

I think Richard has had sufficient time to comment. You should be fine to commit and we can deal with any problems post-commit.

Thanks, Aaron!

This revision was automatically updated to reflect the committed changes.