[Patch][LoopVectorize]Late evaluation of vectorization requirements

[Hal Finkel via llvm-commits]

Hi Tyler,

These LGTM, thanks!

 -Hal

----- Original Message -----
> From: "Tyler Nowicki" <tnowicki at apple.com>
> To: "Hal J. Finkel" <hfinkel at anl.gov>, "Gerolf Hoflehner" <ghoflehner at apple.com>
> Cc: "Commit Messages and Patches for LLVM" <llvm-commits at lists.llvm.org>, "llvm cfe" <cfe-commits at lists.llvm.org>
> Sent: Thursday, August 6, 2015 3:12:29 PM
> Subject: Re: [Patch][LoopVectorize]Late evaluation of vectorization	requirements
> 
> 
> I’ve updated the patches a bit. I am going post another pair of
> patches to add another late diagnostic soon as well.
> 
> 
> Please review,
> 
> 
> Tyler
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> On Aug 5, 2015, at 1:57 PM, Tyler Nowicki < tnowicki at apple.com >
> wrote:
> 
> 
> Hi,
> 
> 
> Could I get a review of these patches?
> 
> 
> Thanks,
> 
> 
> Tyler
> 
> 
> 
> 
> 
> 
> 
> On Jul 27, 2015, at 4:45 PM, Tyler Nowicki < tnowicki at apple.com >
> wrote:
> 
> Please ignore the debug line in the LLVM late-evaluation patch. It
> won’t be part of the commit.
> 
> 
> + DEBUG(dbgs() << "LV: Emitting analysis message.\n”);
> 
> 
> Tyler
> 
> 
> 
> 
> 
> On Jul 27, 2015, at 3:23 PM, Tyler Nowicki < tnowicki at apple.com >
> wrote:
> 
> 
> 
> Hi Hal,
> 
> 
> Thanks for the review! No worries about the delay.
> 
> 
> 
> 
> 
> 
> 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?\
> 
> 
> 
> The diagnostic handling code doesn’t use isa or dyn_cast, rather it
> uses switches to select between different types. I modified the
> patch to use a subclass rather than a member variable. Let me know
> what patch you think would work out better?
> 
> 
> 
> 
> 
> 
> 
> 
> 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.
> 
> So using -O2 or using -fvectorize does not help?
> 
> 
> 
> Using -O1 with -fvectorize seems to work, at least it is a smaller
> set of passes than O3.
> 
> 
> I attached the updated patches. I also noticed that
> DiagnosticInfoOptimizationBase::classof() was incorrectly
> implemented. It would need its own diagnostic kind, but that doesn’t
> make sense because you would never instantiate the base class. I
> thought it was best just to remove it. See the third patch.
> 
> 
> Tyler
> 
> 
> <LLVM-Late-evaluation-of-vectorization-requirements.patch>
> 
> <Clang-Add-diagnostic-to-append-fp-commute-clang-options-to.patch>
> 
> <LLVM-Removed-unused-and-broken-classoff-method-in-base-cl.patch>
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory