<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Thanks!<div class=""><br class=""></div><div class="">Committed in clang r244492, llvm r244489/r244505/r244494.</div><span class=""><br class=""></span><div class=""><font face="Menlo" class=""><span style="font-size: 11px;" class="">Tyler</span></font></div><div class=""><font face="Menlo" class=""><span style="font-size: 11px;" class=""><br class=""></span></font><div><blockquote type="cite" class=""><div class="">On Aug 9, 2015, at 9:33 PM, Hal Finkel <<a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">Hi Tyler,<br class=""><br class="">These LGTM, thanks!<br class=""><br class=""> -Hal<br class=""><br class="">----- Original Message -----<br class=""><blockquote type="cite" class="">From: "Tyler Nowicki" <<a href="mailto:tnowicki@apple.com" class="">tnowicki@apple.com</a>><br class="">To: "Hal J. Finkel" <<a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a>>, "Gerolf Hoflehner" <<a href="mailto:ghoflehner@apple.com" class="">ghoflehner@apple.com</a>><br class="">Cc: "Commit Messages and Patches for LLVM" <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>>, "llvm cfe" <<a href="mailto:cfe-commits@lists.llvm.org" class="">cfe-commits@lists.llvm.org</a>><br class="">Sent: Thursday, August 6, 2015 3:12:29 PM<br class="">Subject: Re: [Patch][LoopVectorize]Late evaluation of vectorization<span class="Apple-tab-span" style="white-space:pre"> </span>requirements<br class=""><br class=""><br class="">I’ve updated the patches a bit. I am going post another pair of<br class="">patches to add another late diagnostic soon as well.<br class=""><br class=""><br class="">Please review,<br class=""><br class=""><br class="">Tyler<br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class="">On Aug 5, 2015, at 1:57 PM, Tyler Nowicki < <a href="mailto:tnowicki@apple.com" class="">tnowicki@apple.com</a> ><br class="">wrote:<br class=""><br class=""><br class="">Hi,<br class=""><br class=""><br class="">Could I get a review of these patches?<br class=""><br class=""><br class="">Thanks,<br class=""><br class=""><br class="">Tyler<br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class="">On Jul 27, 2015, at 4:45 PM, Tyler Nowicki < <a href="mailto:tnowicki@apple.com" class="">tnowicki@apple.com</a> ><br class="">wrote:<br class=""><br class="">Please ignore the debug line in the LLVM late-evaluation patch. It<br class="">won’t be part of the commit.<br class=""><br class=""><br class="">+ DEBUG(dbgs() << "LV: Emitting analysis message.\n”);<br class=""><br class=""><br class="">Tyler<br class=""><br class=""><br class=""><br class=""><br class=""><br class="">On Jul 27, 2015, at 3:23 PM, Tyler Nowicki < <a href="mailto:tnowicki@apple.com" class="">tnowicki@apple.com</a> ><br class="">wrote:<br class=""><br class=""><br class=""><br class="">Hi Hal,<br class=""><br class=""><br class="">Thanks for the review! No worries about the delay.<br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class="">Could I get a review of these patches for cfe and llvm?<br class=""><br class="">Hi Tyler,<br class=""><br class="">I'm apologize for the delay. I think this generally looks good, but I<br class="">don't understand the motivation for introducing the additional<br class="">FrontendOptions member. Why not just make a subclass of<br class="">DiagnosticInfoOptimizationRemarkAnalysis that the frontend can<br class="">handle specially (and detect using the normal isa/dyn_cast<br class="">mechanism?\<br class=""><br class=""><br class=""><br class="">The diagnostic handling code doesn’t use isa or dyn_cast, rather it<br class="">uses switches to select between different types. I modified the<br class="">patch to use a subclass rather than a member variable. Let me know<br class="">what patch you think would work out better?<br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class=""><br class="">I should have also said in my previous email that I am not thrilled<br class="">by the need to use O3 in the clang-side test.<br class=""><br class="">So using -O2 or using -fvectorize does not help?<br class=""><br class=""><br class=""><br class="">Using -O1 with -fvectorize seems to work, at least it is a smaller<br class="">set of passes than O3.<br class=""><br class=""><br class="">I attached the updated patches. I also noticed that<br class="">DiagnosticInfoOptimizationBase::classof() was incorrectly<br class="">implemented. It would need its own diagnostic kind, but that doesn’t<br class="">make sense because you would never instantiate the base class. I<br class="">thought it was best just to remove it. See the third patch.<br class=""><br class=""><br class="">Tyler<br class=""><br class=""><br class=""><LLVM-Late-evaluation-of-vectorization-requirements.patch><br class=""><br class=""><Clang-Add-diagnostic-to-append-fp-commute-clang-options-to.patch><br class=""><br class=""><LLVM-Removed-unused-and-broken-classoff-method-in-base-cl.patch><br class=""><br class=""><br class="">_______________________________________________<br class="">cfe-commits mailing list<br class=""><a href="mailto:cfe-commits@cs.uiuc.edu" class="">cfe-commits@cs.uiuc.edu</a><br class="">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits<br class=""><br class=""><br class=""></blockquote><br class="">-- <br class="">Hal Finkel<br class="">Assistant Computational Scientist<br class="">Leadership Computing Facility<br class="">Argonne National Laboratory<br class=""></div></blockquote></div><br class=""></div></body></html>