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
Description: Binary data
LLVM-Late-evaluation-of-vectorization-requirements.patch
Description: Binary data
> On Jul 9, 2015, at 7:09 PM, Hal Finkel <hfin...@anl.gov> wrote:
>
> ----- Original Message -----
>> From: "Tyler Nowicki" <tnowi...@apple.com>
>> To: "Commit Messages and Patches for LLVM" <llvm-comm...@cs.uiuc.edu>
>> Cc: "Gerolf Hoflehner" <ghofleh...@apple.com>, "Hal J. Finkel"
>> <hfin...@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 < tnowi...@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
_______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
