<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=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jan 27, 2015, at 11:44 AM, Hal Finkel <<a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">----- Original Message -----</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">From: "Michael Zolotukhin" <<a href="mailto:mzolotukhin@apple.com" class="">mzolotukhin@apple.com</a>><br class="">To: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a>><br class="">Cc: "Commit Messages and Patches for LLVM" <<a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a>><br class="">Sent: Tuesday, January 27, 2015 1:36:14 PM<br class="">Subject: Re: Add detailed diagnostics to LoopUnroll pass<br class=""><br class=""><br class=""><blockquote type="cite" class="">On Jan 27, 2015, at 6:07 AM, Hal Finkel <<a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a>> wrote:<br class=""><br class="">----- Original Message -----<br class=""><blockquote type="cite" class="">From: "Michael Zolotukhin" <<a href="mailto:mzolotukhin@apple.com" class="">mzolotukhin@apple.com</a>><br class="">To: "Hal J. Finkel" <<a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a>><br class="">Cc: "Commit Messages and Patches for LLVM"<br class=""><<a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a>><br class="">Sent: Monday, January 26, 2015 4:49:24 PM<br class="">Subject: Add detailed diagnostics to LoopUnroll pass<br class=""><br class=""><br class=""><br class="">Hi,<br class=""><br class="">This patch adds some diagnostics to loop-unroll pass, triggered by<br class="">-Rpass-analysis=loop-unroll and -Rpass-missed=loop-unroll.<br class="">Currently<br class="">we only do -Rpass=loop-unroll diagnostics in this pass.<br class=""><br class=""></blockquote><br class=""> if (HasUnrollDisablePragma(L)) {<br class=""><br class="">+ emitOptimizationRemarkMissed(Ctx, DEBUG_TYPE, *F, LoopLoc,<br class=""><br class="">+ "Unrolling is disabled by<br class="">pragma.");<br class=""><br class="">Please don't add a trailing period to the messages, I don't believe<br class="">that's the convention used by the other passes (this applies to<br class="">all messages).<br class=""><br class="">+ "Unrolling is impossible, because<br class="">the loop "<br class=""><br class="">+ "contains non-duplicable<br class="">instructions.");<br class=""><br class="">Remove comma before 'because'<br class=""><br class="">+ emitOptimizationRemarkMissed(Ctx, DEBUG_TYPE, *F, LoopLoc,<br class=""><br class="">+ "Unrolling is disabled, because<br class="">the loop "<br class=""><br class="">+ "contains candidates for<br class="">inlining.");<br class=""><br class="">Remove comma before 'because'<br class=""><br class="">+ if (TripCount) {<br class=""><br class="">+ if (Count == TripCount &&<br class=""><br class="">+ (Threshold == NoThreshold || UnrolledSize <= Threshold)) {<br class=""><br class="">+ Unrolling = Full;<br class=""><br class="">+ emitOptimizationRemarkAnalysis(Ctx, DEBUG_TYPE, *F, LoopLoc,<br class=""><br class="">+ "Will try to completely<br class="">unroll the loop.");<br class=""><br class="">Saying 'try' here sounds odd. Either the decision has been made or<br class="">it has not. If not, move the message to some later point.<br class="">Otherwise, simple state that the loop will be fully unrolled.<br class=""><br class="">+ emitOptimizationRemarkAnalysis(Ctx, DEBUG_TYPE, *F, LoopLoc,<br class=""><br class="">+ "Loop cannot be completely<br class="">unrolled, "<br class=""><br class="">+ "because it is too large.");<br class=""><br class="">Remove comma before 'because'<br class=""><br class="">+ emitOptimizationRemarkAnalysis(Ctx, DEBUG_TYPE, *F, LoopLoc,<br class=""><br class="">+ "Loop tripcount: " +<br class="">Twine(TripCount) +<br class=""><br class="">+ ". Estimated unrolled size: " +<br class=""><br class="">+ Twine(UnrolledSize) +<br class=""><br class="">+ ". Threshold: " +<br class="">Twine(Threshold) + ".");<br class=""><br class="">I'm not sure the size information is appropriate for end users<br class="">(from their perspective, these are in arbitrary units); I think<br class="">leaving this in the debug information is sufficient. Please make<br class="">trip count two words.<br class=""><br class="">+ emitOptimizationRemarkAnalysis(Ctx, DEBUG_TYPE, *F, LoopLoc,<br class=""><br class="">+ "Loop tripcount is unknown,<br class="">only runtime "<br class=""><br class="">+ "unrolling is possible.");<br class=""><br class="">Trip count should be two words<br class=""><br class="">+ emitOptimizationRemarkMissed(Ctx, DEBUG_TYPE, *F, LoopLoc,<br class=""><br class="">+ "Cannot unroll because loop has<br class="">a runtime "<br class=""><br class="">+ "trip count.");<br class=""><br class="">We also have to be careful here not to produce messaging that users<br class="">will find confusing. The unroller is run with partial/runtime<br class="">unrolling disabled many times (iteratively within the<br class="">inliner-driven CGSCC pass manager), and the user might end up<br class="">seeing these messages many times for the same loop, which would be<br class="">confusing, and also confusing because 'disabled' is not really<br class="">true but rather delayed until the end of the pipeline.<br class=""></blockquote>Hmm.. that’s true. Indeed we emit the messages several times for one<br class="">loop. Now I’m not sure if we need such diagnostics at all.<br class="">Personally I found it useful, but other users, especially not<br class="">involved into LLVM development, might have a different opinion on<br class="">that. Do you think it’s worth adding these messages at all?<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">I recommend leaving them out for now; we can revisit this later if we'd like.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote>Okay, that makes sense. Thank you for the comments and review!</div><div><br class=""></div><div>Michael<br class=""><blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">-Hal</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class="">Thanks,<br class="">Michael<br class=""><blockquote type="cite" class=""><br class="">As a result, we should really not say anything about these loops<br class="">that require runtime/partial unrolling until the "final" run of<br class="">the unroller at the end of the pipeline.<br class=""><br class="">Thanks again,<br class="">Hal<br class=""><br class=""><blockquote type="cite" class=""><br class=""><br class=""><br class=""><br class="">The patch shouldn't change any logic in the pass work, only<br class="">rearrange<br class="">code a bit and add the reports. Is it ok to commit?<br class=""><br class="">Thanks,<br class="">Michael<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=""></blockquote><br class=""><br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">--<span class="Apple-converted-space"> </span></span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Hal Finkel</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Assistant Computational Scientist</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Leadership Computing Facility</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Argonne National Laboratory</span></div></blockquote></div><br class=""></body></html>