[PATCH] D66199: [docs] loop pragmas

2019-10-14 Thread Sjoerd Meijer via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG52bfa73af841: [docs] loop pragmas: options implying 
transformations (authored by SjoerdMeijer).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66199/new/

https://reviews.llvm.org/D66199

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -3027,6 +3027,14 @@
 distribution. Loop hints can be specified before any loop and will be ignored 
if
 the optimization is not safe to apply.
 
+There are loop hints that control transformations (e.g. vectorization, loop
+unrolling) and there are loop hints that set transformation options (e.g.
+``vectorize_width``, ``unroll_count``).  Pragmas setting transformation options
+imply the transformation is enabled, as if it was enabled via the corresponding
+transformation pragma (e.g. ``vectorize(enable)``). If the transformation is
+disabled  (e.g. ``vectorize(disable)``), that takes precedence over
+transformations option pragmas implying that transformation.
+
 Vectorization, Interleaving, and Predication
 
 


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -3027,6 +3027,14 @@
 distribution. Loop hints can be specified before any loop and will be ignored if
 the optimization is not safe to apply.
 
+There are loop hints that control transformations (e.g. vectorization, loop
+unrolling) and there are loop hints that set transformation options (e.g.
+``vectorize_width``, ``unroll_count``).  Pragmas setting transformation options
+imply the transformation is enabled, as if it was enabled via the corresponding
+transformation pragma (e.g. ``vectorize(enable)``). If the transformation is
+disabled  (e.g. ``vectorize(disable)``), that takes precedence over
+transformations option pragmas implying that transformation.
+
 Vectorization, Interleaving, and Predication
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66199: [docs] loop pragmas

2019-10-10 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment.

Sure, will do, thanks again for taking a look.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66199/new/

https://reviews.llvm.org/D66199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66199: [docs] loop pragmas

2019-10-10 Thread Hideki Saito via Phabricator via cfe-commits
hsaito accepted this revision.
hsaito added a comment.
This revision is now accepted and ready to land.

LGTM. Please wait for a few days in case others have more comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66199/new/

https://reviews.llvm.org/D66199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66199: [docs] loop pragmas

2019-10-10 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer updated this revision to Diff 224421.
SjoerdMeijer added a comment.

Thanks! Typo fixed.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66199/new/

https://reviews.llvm.org/D66199

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -3027,6 +3027,14 @@
 distribution. Loop hints can be specified before any loop and will be ignored 
if
 the optimization is not safe to apply.
 
+There are loop hints that control transformations (e.g. vectorization, loop
+unrolling) and there are loop hints that set transformation options (e.g.
+``vectorize_width``, ``unroll_count``).  Pragmas setting transformation options
+imply the transformation is enabled, as if it was enabled via the corresponding
+transformation pragma (e.g. ``vectorize(enable)``). If the transformation is
+disabled  (e.g. ``vectorize(disable)``), that takes precedence over
+transformations option pragmas implying that transformation.
+
 Vectorization, Interleaving, and Predication
 
 


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -3027,6 +3027,14 @@
 distribution. Loop hints can be specified before any loop and will be ignored if
 the optimization is not safe to apply.
 
+There are loop hints that control transformations (e.g. vectorization, loop
+unrolling) and there are loop hints that set transformation options (e.g.
+``vectorize_width``, ``unroll_count``).  Pragmas setting transformation options
+imply the transformation is enabled, as if it was enabled via the corresponding
+transformation pragma (e.g. ``vectorize(enable)``). If the transformation is
+disabled  (e.g. ``vectorize(disable)``), that takes precedence over
+transformations option pragmas implying that transformation.
+
 Vectorization, Interleaving, and Predication
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66199: [docs] loop pragmas

2019-10-10 Thread Hideki Saito via Phabricator via cfe-commits
hsaito added inline comments.



Comment at: docs/LanguageExtensions.rst:3069
+There are loop hints that control transformations (e.g. vectorization, loop
+unrolling) and there loop hints that set transformation options (e.g.
+``vectorize_width``, ``unroll_count``).  Pragmas setting transformation options

and there loop hints ==> and there are loop hints


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66199/new/

https://reviews.llvm.org/D66199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66199: [docs] loop pragmas

2019-10-10 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment.

I have commit all my pragma patches, so now back to the last bit, this doc 
update. 
This doc change should now reflect our implementation. Are we happy for this to 
go in?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66199/new/

https://reviews.llvm.org/D66199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66199: [docs] loop pragmas

2019-08-27 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment.

Many thanks for your clarification!

>   What we were discussing was that these settings would remove 0) from the 
> candidate list as well.

Yep, that's crystal clear now.
And my expectation would indeed be that this would be the case.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66199/new/

https://reviews.llvm.org/D66199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66199: [docs] loop pragmas

2019-08-14 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added inline comments.



Comment at: docs/LanguageExtensions.rst:3068-3069
 
+There are loop hints that control transformations (e.g. vectorization, loop
+unrolling) and there loop hints that set transformation options (e.g.
+``vectorize_width``, ``unroll_count``).  Pragmas setting transformation options

SjoerdMeijer wrote:
> Meinersbur wrote:
> > `vectorize_width` also "controls" a transformation. Is it that in our 
> > interpretation, `vectorize(enable)` overrides the profitability heuristic 
> > by taking out the "do not apply at all" option without setting any other 
> > option?
> I think I need to think a little bit more about this:
> 
> > Is it that in our interpretation, vectorize(enable) overrides the 
> > profitability heuristic by taking out the "do not apply at all" option 
> > without setting any other option?
> 
> but I would answer this with "yes". But just checking, what exactly do you 
> mean with "do not apply at all option"?
I unfortunately mixed two meanings of "option" here:
1. option as in optimization setting
2. option as in candidate to choose from.

Let's take a look on the loop vectorizer. The profitability heuristic's job is 
too choose the "best" candidate out of the following list (independently of how 
LoopVectorize's heuristic is actually implemented; these might be decision made 
independently of each other, but as the user still sees one of the following 
outcomes):

0) no vectorization (the "do not apply at all" option/candidate)
1a) vec width=2 with epilogue
1b) vec width=2 without epilogue
2a) vec width=4 with epilogue
2b) vec width=4 without epilogue
3a) vec width=8 with epilogue
3b) vec width=8 without epilogue
...

`vectorize(enable)` with remove 0) as a candidate. `vecorize(disable)` will 
remove all expect 0) as a candidate. 

The option/settings are `vectorize_width` and `vectorize_predicate` (and 
`interleave_count`). For instance `vectorize_width(4)` would remove 1) and 3)+ 
from the candidate list. `vectorize_predicate(enable)` takes any a) candidate 
from the list. What we were discussing was that these settings would remove 0) 
from the candidate list as well.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66199/new/

https://reviews.llvm.org/D66199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66199: [docs] loop pragmas

2019-08-14 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer marked an inline comment as done.
SjoerdMeijer added a comment.

> Since this is user documentation, we should only add it here once it is true.

Yep, good point, I also wanted to capture our discussions on the list. But yes, 
let's hold this back until we're ready with the pragmas.




Comment at: docs/LanguageExtensions.rst:3068-3069
 
+There are loop hints that control transformations (e.g. vectorization, loop
+unrolling) and there loop hints that set transformation options (e.g.
+``vectorize_width``, ``unroll_count``).  Pragmas setting transformation options

Meinersbur wrote:
> `vectorize_width` also "controls" a transformation. Is it that in our 
> interpretation, `vectorize(enable)` overrides the profitability heuristic by 
> taking out the "do not apply at all" option without setting any other option?
I think I need to think a little bit more about this:

> Is it that in our interpretation, vectorize(enable) overrides the 
> profitability heuristic by taking out the "do not apply at all" option 
> without setting any other option?

but I would answer this with "yes". But just checking, what exactly do you mean 
with "do not apply at all option"?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66199/new/

https://reviews.llvm.org/D66199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66199: [docs] loop pragmas

2019-08-14 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

Since this is user documentation, we should only add it here once it is true.




Comment at: docs/LanguageExtensions.rst:3068-3069
 
+There are loop hints that control transformations (e.g. vectorization, loop
+unrolling) and there loop hints that set transformation options (e.g.
+``vectorize_width``, ``unroll_count``).  Pragmas setting transformation options

`vectorize_width` also "controls" a transformation. Is it that in our 
interpretation, `vectorize(enable)` overrides the profitability heuristic by 
taking out the "do not apply at all" option without setting any other option?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66199/new/

https://reviews.llvm.org/D66199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66199: [docs] loop pragmas

2019-08-14 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: fhahn, Meinersbur, dorit, hsaito.

Following our discussion on the cfe dev list
(http://lists.llvm.org/pipermail/cfe-dev/2019-August/063054.html), I have added
a paragraph that is explicit about transformation options implying
the corresponding transformation.


https://reviews.llvm.org/D66199

Files:
  docs/LanguageExtensions.rst


Index: docs/LanguageExtensions.rst
===
--- docs/LanguageExtensions.rst
+++ docs/LanguageExtensions.rst
@@ -3065,6 +3065,14 @@
 distribution. Loop hints can be specified before any loop and will be ignored 
if
 the optimization is not safe to apply.
 
+There are loop hints that control transformations (e.g. vectorization, loop
+unrolling) and there loop hints that set transformation options (e.g.
+``vectorize_width``, ``unroll_count``).  Pragmas setting transformation options
+imply the transformation is enabled, as if it was enabled via the corresponding
+transformation pragma (e.g. ``vectorize(enable)``). If the transformation is
+disabled  (e.g. ``vectorize(disable)``), that takes precedence over
+transformations option pragmas implying that transformation.
+
 Vectorization, Interleaving, and Predication
 
 


Index: docs/LanguageExtensions.rst
===
--- docs/LanguageExtensions.rst
+++ docs/LanguageExtensions.rst
@@ -3065,6 +3065,14 @@
 distribution. Loop hints can be specified before any loop and will be ignored if
 the optimization is not safe to apply.
 
+There are loop hints that control transformations (e.g. vectorization, loop
+unrolling) and there loop hints that set transformation options (e.g.
+``vectorize_width``, ``unroll_count``).  Pragmas setting transformation options
+imply the transformation is enabled, as if it was enabled via the corresponding
+transformation pragma (e.g. ``vectorize(enable)``). If the transformation is
+disabled  (e.g. ``vectorize(disable)``), that takes precedence over
+transformations option pragmas implying that transformation.
+
 Vectorization, Interleaving, and Predication
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits