Hi Tyler, These LGTM, thanks!
-Hal ----- Original Message ----- > From: "Tyler Nowicki" <tnowi...@apple.com> > To: "Hal J. Finkel" <hfin...@anl.gov>, "Gerolf Hoflehner" > <ghofleh...@apple.com> > Cc: "Commit Messages and Patches for LLVM" <llvm-comm...@lists.llvm.org>, > "llvm cfe" <cfe-commits@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 < tnowi...@apple.com > > wrote: > > > Hi, > > > Could I get a review of these patches? > > > Thanks, > > > Tyler > > > > > > > > On Jul 27, 2015, at 4:45 PM, Tyler Nowicki < tnowi...@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 < tnowi...@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-comm...@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > -- Hal Finkel Assistant Computational Scientist Leadership Computing Facility Argonne National Laboratory
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
