[Patch][LoopVectorize]Late evaluation of vectorization requirements
Hal Finkel
hfinkel at anl.gov
Sat Jul 25 19:33:53 PDT 2015
----- Original Message -----
> From: "Tyler Nowicki" <tnowicki at apple.com>
> To: "Hal Finkel" <hfinkel at anl.gov>, "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>, "llvm cfe"
> <cfe-commits at cs.uiuc.edu>
> Cc: "Gerolf Hoflehner" <ghoflehner at apple.com>
> Sent: Monday, July 20, 2015 1:40:28 PM
> Subject: Re: [Patch][LoopVectorize]Late evaluation of vectorization requirements
>
> Hi,
>
>
> Could I get a review of these patches for cfe and llvm?
Hi Tyler,
I'm apologize for the delay. I think this generally looks good, but I don't understand the motivation for introducing the additional FrontendOptions member. Why not just make a subclass of DiagnosticInfoOptimizationRemarkAnalysis that the frontend can handle specially (and detect using the normal isa/dyn_cast mechanism?
>
>
> I should have also said in my previous email that I am not thrilled
> by the need to use O3 in the clang-side test. I need the vectorizer
> to generate the backend diagnostic message so the front-end can
> append the appropriate compiler options. The clang-side test
> verifies that the correct compiler options are being appended to the
> backend diagnostic message that informs the user of the fp commute
> issue.
>
>
> I have a llvm-side test to verify the backend diagnostic message is
> generated correctly. I need the clang-side test to verify the
> front-end recognizes the message and appends the correct compiler
> options. The inliner is tested in a similar way in the front-end
> without O3 because it is triggered with cc1 without any command line
> options. Unfortunately the command line options described in '
> http://llvm.org/docs/Vectorizers.html' don’t seem to trigger the
> vectorizer without O3.
So using -O2 or using -fvectorize does not help?
-Hal
>
>
> Thanks,
>
>
> Tyler
>
>
>
>
>
> On Jul 15, 2015, at 3:55 PM, Tyler Nowicki < tnowicki at apple.com >
> wrote:
>
> Hi Hal,
>
> Thanks of the review! I split the message moving the front-end
> specific parts into clang. Rather than using a special subclass for
> this message I used the existing subclass
> DiagnosticOptimizationRemarkAnalysis and added a parameter for the
> category of front-end options that should be appended to the
> message. In the future I will extend this to include several other
> categories of analysis messages.
>
> I didn’t put the category parameter higher in the subclass hierarchy
> (for example in DiagnosticOptimizationRemark) because I think it
> only makes sense for analysis remarks to be followed by a list of
> front-end specific options.
>
> [Clang Patch]
> - Select diagnostic message based on the options category.
>
> [LLVM Patch]
> - Specified options category when generating diagnostic message.
>
> Tyler
>
> <Clang-Add-diagnostic-to-append-fp-commute-clang-options.patch>
> <LLVM-Late-evaluation-of-vectorization-requirements.patch>
>
>
>
>
> On Jul 9, 2015, at 7:09 PM, Hal Finkel < hfinkel at anl.gov > wrote:
>
> ----- Original Message -----
>
>
> From: "Tyler Nowicki" < tnowicki at apple.com >
> To: "Commit Messages and Patches for LLVM" < llvm-commits at cs.uiuc.edu
> >
> Cc: "Gerolf Hoflehner" < ghoflehner at apple.com >, "Hal J. Finkel" <
> hfinkel at anl.gov >
> Sent: Sunday, June 28, 2015 3:17:01 PM
> Subject: Re: [Patch][LoopVectorize]Late evaluation of vectorization
> requirements
>
>
>
> Improved patch with clang-format.
>
>
> Tyler
>
>
> On Jun 28, 2015, at 1:09 PM, Tyler Nowicki < tnowicki at apple.com >
> wrote:
>
>
> Hi,
>
>
>
> To vectorize fp reductions fast-math is required, but this check is
> done while identifying the reduction causing a very cryptical
> analysis message. Also it should be possible to provide a loop hint
> rather than specifying fast-math for the whole module.
>
>
>
> For float reductions the current analysis message reads:
> - value that could not be identified as reduction is used outside the
> loop
>
>
> This patch moves the verification of fast-math, adds a check for a
> loop hint, and improves the analysis message.
>
>
> The improved message reads:
> - vectorization requires changes in the order of operations, however
> IEEE 754 floating-point operations are not commutative; allow
> commutativity by specifying ‘#pragma clang loop vectorize(enable)’
> before the loop or by providing the compiler option ‘-ffast-math’
>
>
> + "commutative; allow commutativity by specifying ‘#pragma clang "
> + "loop vectorize(enable)’ before the loop or by providing the "
> + "compiler option ‘-ffast-math’";
> + }
> + llvm_unreachable("Unknown requirement error");
>
> I agree that this is the best kind of message, but Clang is not the
> only LLVM frontend, and we should not talk about Clang options here.
> When we designed the diagnostic system, we specifically gave it a
> class hierarchy so that the frontend could treat some messages
> specially, and this seems like a perfect example of where we should
> do so (by having a special subclass for these kinds of vectorizer
> messages, and having Clang do special things with them).
>
> -Hal
>
>
>
>
>
> In the future I will extend this to include pointer aliasing checks
> and perhaps cost-model checks.
>
>
> This patch builds off of my other diagnostic changes that are
> currently in review ( http://reviews.llvm.org/D10714 ) and “Renaming
> and Diagnostics for Loop Interleaving". Please make sure to apply
> patches 0002 and 0003 if you want to try this patch.
>
>
> Comments are much appreciated!
>
>
> Tyler
>
>
> <0004-Late-evaluation-of-vectorization-requirements.patch>
>
>
>
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
>
>
>
--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory
More information about the cfe-commits
mailing list