Hi,

Could I get a review of these patches for cfe and llvm?

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' 
<http://llvm.org/docs/Vectorizers.html'> don’t seem to trigger the vectorizer 
without O3.

Thanks,

Tyler

> On Jul 15, 2015, at 3:55 PM, Tyler Nowicki <tnowi...@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 <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

Reply via email to