Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add loop pragmas #31376

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add loop pragmas #31376

wants to merge 1 commit into from

Conversation

vchuravy
Copy link
Sponsor Member

based on #31347, the loopinfo Expr allows us to communicate various pieces of information about loops to LLVM, in the C/OpenMP world these are know as loop pragmas.
They are optional metadata and LLVM can choose to discard it so it is not the best mechanism.

Mostly putting this up so that we can discuss the user-facing interface (which I have no strong feelings about.)

@vchuravy vchuravy added needs tests Unit tests are required for this change needs docs Documentation for this change is required needs news A NEWS entry is required for this change labels Mar 16, 2019
@vchuravy vchuravy changed the base branch from vc/loopinfo2 to master March 26, 2019 20:54
@DilumAluthge
Copy link
Member

@vchuravy Can you rebase this on the latest master?

@DilumAluthge
Copy link
Member

It looks like this PR has tests now, so I'm removing the "needs tests" label.

@DilumAluthge DilumAluthge removed the needs tests Unit tests are required for this change label Jan 12, 2021
@DilumAluthge DilumAluthge marked this pull request as ready for review January 12, 2021 20:51
@vchuravy
Copy link
Sponsor Member Author

I think the big picture question here is what the user interface is supposed to be. I don't think we should have indivdual macros for each variant, and how does one note multiple loop pragmas at once?

@vchuravy
Copy link
Sponsor Member Author

Also documentation can be found here https://llvm.org/docs/TransformMetadata.html and here https://clang.llvm.org/docs/LanguageExtensions.html#extensions-for-loop-hint-optimizations

@DilumAluthge
Copy link
Member

Personally I don't mind the "individual macros" approach. But we could have a single macro @loopinfo as such:

Individual macro Single macro
@unroll for @loopinfo unroll for
@unroll 3 for @loopinfo unroll=3 for
@jam for @loopinfo jam for
@jam 5 for @loopinfo jam=5 for

@DilumAluthge
Copy link
Member

DilumAluthge commented Jan 12, 2021

Btw, the LLVM docs don't quite match what we have in this PR? E.g. in this PR, @jam 3 for will set llvm.loop.unroll_and_jam.count to 3.

But https://llvm.org/docs/TransformMetadata.html does not mention llvm.loop.unroll_and_jam.count?

Presumably it sets the unroll factor. But it's not in the docs.

@vchuravy
Copy link
Sponsor Member Author

It's not mentioned there, but is the IR version of @pargma unroll_and_jam(4). With LLVM the rule is always trust the source ;)

@StefanKarpinski
Copy link
Sponsor Member

Needs API design work to make progress.

@carstenbauer
Copy link
Member

FWIW, I would really like to see this merged. Anything I can help with?

@vchuravy
Copy link
Sponsor Member Author

I am really not happy with the UX design, but I haven't had any better ideas. You can see that I only exposed a few and then got frustrated with the plethora of names. There is also a compositionality question. E.g. @unroll @jam what does that mean can we support. So maybe we need to do something more like a @pragma :unroll...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs docs Documentation for this change is required needs news A NEWS entry is required for this change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants