<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Hi,</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Sure, I won't mind fixing these other cases too while I am at it.<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
I think my only preference would be to make one functional change at a time (fix one pragma (option) at a time), because this has turned into a little project  on itself and dealing with issues would be easier, unless a big-bang change would be preferred in
 this case for some reason and then I will address them all in D65776. <br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
I think one thing we haven't discussed yet is conflicting combinations like:</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
    vectorize(enable) vectorize_width(1)</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Should this enable of disable vectorisation? This is now important when an option implies setting the transformation.  I think it probably makes most sense that enabling/disabling the transformation should take precedence over setting the transformation options,
 and so in this example vectorisation should be enabled. With this logic, the predicate hint is ignored because vectorisation is disabled in this case:<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<span>    vectorize(disable) vectorize_predicate(enable)</span><br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
which is what is being proposed in D65776.</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
After fixing the Clang behaviour, it would be good to revise the LLVM parts that check these transformations and options, to check things are consistent or can be simplified like the code example in the first message.<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Cheers,</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Sjoerd.<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Michael Kruse <llvm@meinersbur.de><br>
<b>Sent:</b> 12 August 2019 22:42<br>
<b>To:</b> Sjoerd Meijer <Sjoerd.Meijer@arm.com><br>
<b>Cc:</b> cfe-dev@lists.llvm.org <cfe-dev@lists.llvm.org>; nd <nd@arm.com><br>
<b>Subject:</b> Re: [cfe-dev] Behaviour of loop pragmas and transformation options</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">Hi,<br>
<br>
if the consensus is that `vectorize_predicate()` should also set<br>
`llvm.loop.vectorize.enable`, could you change the patch to apply this<br>
also to `vectorize_width()`, `interleave_count()`?<br>
<br>
I don think we need to change unroll_count() atm.<br>
<br>
Michael<br>
<br>
<br>
<br>
<br>
Am Mi., 7. Aug. 2019 um 02:25 Uhr schrieb Sjoerd Meijer via cfe-dev<br>
<cfe-dev@lists.llvm.org>:<br>
><br>
> We have recently added support for a new loop pragma 'vectorize_predicate' [1].<br>
> During review of the LLVM part that does the actual transformation [2], the<br>
> question was raised if 'vectorize_predicate' should imply 'vectorize(enable)'.<br>
> I agree that this is indeed what users would most likely expect and so I<br>
> created a patch for that [3].<br>
><br>
> During review of [3], Florian mentioned bug report [4] supporting the case that<br>
> users indeed expect that more specific pragmas such as e.g. vectorize_width(4)<br>
> should imply vectorize(enable), but this is currently not the case.  But our<br>
> docs for example mention: "The following example implicitly enables<br>
> vectorization and interleaving by specifying a vector width and interleaving<br>
> count", and so we have an inconsistency in the docs and implementation. But as<br>
> I have e.g. no clue what the behaviour could be of "vectorize(disable)<br>
> vectorize_predicate(enable)", I would say the docs are right and the<br>
> implementation is wrong.<br>
><br>
> Michael mentioned that the situation with for example loop unroll pragmas<br>
> 'unroll(enable) unroll_count(4)' is not very different. Pragma 'unroll_count'<br>
> also does not set 'llvm.loop.unroll.enable', but it is handled by the<br>
> LoopUnroll pass itself and so we end up with checks like this:<br>
><br>
>     bool ExplicitUnroll = PragmaCount > 0 || PragmaFullUnroll ||<br>
>                           PragmaEnableUnroll || UserUnrollCount;<br>
><br>
> And again, this is very similar to 'vectorize_width', and we have different<br>
> checks to see if vectorization is enabled.<br>
><br>
> Because of the many inconsistencies, we came to the conclusion in [3] that we<br>
> don't know how know how to (best) implement that setting an transformation<br>
> option implies setting the transformation.<br>
><br>
> My proposal would be that the transformation options imply the transformations<br>
> by setting the enable flag of that transformation. Thus, vectorize options<br>
> vectorize_width, vectorize_predicate, etc., set vectorize(enable), and unroll<br>
> option unroll count set unroll(enable).<br>
><br>
> Any thoughts on this welcome.<br>
><br>
><br>
> [1] <a href="https://reviews.llvm.org/rL366989">https://reviews.llvm.org/rL366989</a><br>
> [2] <a href="https://reviews.llvm.org/D65197">https://reviews.llvm.org/D65197</a><br>
> [3] <a href="https://reviews.llvm.org/D65776">https://reviews.llvm.org/D65776</a><br>
> [4] <a href="https://bugs.llvm.org/show_bug.cgi?id=27643">https://bugs.llvm.org/show_bug.cgi?id=27643</a><br>
> _______________________________________________<br>
> cfe-dev mailing list<br>
> cfe-dev@lists.llvm.org<br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</div>
</span></font></div>
</body>
</html>