[cfe-dev] Behaviour of loop pragmas and transformation options

[Sjoerd Meijer via cfe-dev]

Hi,

Sure, I won't mind fixing these other cases too while I am at it.
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.

I think one thing we haven't discussed yet is conflicting combinations like:

    vectorize(enable) vectorize_width(1)

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:

    vectorize(disable) vectorize_predicate(enable)

which is what is being proposed in D65776.

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.

Cheers,
Sjoerd.

________________________________
From: Michael Kruse <llvm at meinersbur.de>
Sent: 12 August 2019 22:42
To: Sjoerd Meijer <Sjoerd.Meijer at arm.com>
Cc: cfe-dev at lists.llvm.org <cfe-dev at lists.llvm.org>; nd <nd at arm.com>
Subject: Re: [cfe-dev] Behaviour of loop pragmas and transformation options

Hi,

if the consensus is that `vectorize_predicate()` should also set
`llvm.loop.vectorize.enable`, could you change the patch to apply this
also to `vectorize_width()`, `interleave_count()`?

I don think we need to change unroll_count() atm.

Michael




Am Mi., 7. Aug. 2019 um 02:25 Uhr schrieb Sjoerd Meijer via cfe-dev
<cfe-dev at lists.llvm.org>:
>
> We have recently added support for a new loop pragma 'vectorize_predicate' [1].
> During review of the LLVM part that does the actual transformation [2], the
> question was raised if 'vectorize_predicate' should imply 'vectorize(enable)'.
> I agree that this is indeed what users would most likely expect and so I
> created a patch for that [3].
>
> During review of [3], Florian mentioned bug report [4] supporting the case that
> users indeed expect that more specific pragmas such as e.g. vectorize_width(4)
> should imply vectorize(enable), but this is currently not the case.  But our
> docs for example mention: "The following example implicitly enables
> vectorization and interleaving by specifying a vector width and interleaving
> count", and so we have an inconsistency in the docs and implementation. But as
> I have e.g. no clue what the behaviour could be of "vectorize(disable)
> vectorize_predicate(enable)", I would say the docs are right and the
> implementation is wrong.
>
> Michael mentioned that the situation with for example loop unroll pragmas
> 'unroll(enable) unroll_count(4)' is not very different. Pragma 'unroll_count'
> also does not set 'llvm.loop.unroll.enable', but it is handled by the
> LoopUnroll pass itself and so we end up with checks like this:
>
>     bool ExplicitUnroll = PragmaCount > 0 || PragmaFullUnroll ||
>                           PragmaEnableUnroll || UserUnrollCount;
>
> And again, this is very similar to 'vectorize_width', and we have different
> checks to see if vectorization is enabled.
>
> Because of the many inconsistencies, we came to the conclusion in [3] that we
> don't know how know how to (best) implement that setting an transformation
> option implies setting the transformation.
>
> My proposal would be that the transformation options imply the transformations
> by setting the enable flag of that transformation. Thus, vectorize options
> vectorize_width, vectorize_predicate, etc., set vectorize(enable), and unroll
> option unroll count set unroll(enable).
>
> Any thoughts on this welcome.
>
>
> [1] https://reviews.llvm.org/rL366989
> [2] https://reviews.llvm.org/D65197
> [3] https://reviews.llvm.org/D65776
> [4] https://bugs.llvm.org/show_bug.cgi?id=27643
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20190813/da4c8ea6/attachment.html>