<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
</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 2, 2018, at 10:06 PM, Friedman, Eli <<a href="mailto:efriedma@codeaurora.org" class="">efriedma@codeaurora.org</a>> wrote:</div>
<br class="Apple-interchange-newline">
<div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">On
1/2/2018 3:45 PM, Volkan Keles via llvm-commits wrote:</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
<blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">
+llvm-commits<br class="">
<br class="">
<blockquote type="cite" class="">On Jan 2, 2018, at 8:25 AM, Anna Thomas via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class="">
<br class="">
Author: annat<br class="">
Date: Tue Jan 2 08:25:50 2018<br class="">
New Revision: 321653<br class="">
<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=321653&view=rev" class="">http://llvm.org/viewvc/llvm-project?rev=321653&view=rev</a><br class="">
Log:<br class="">
[BasicBlockUtils] Check for unreachable preds before updating LI in UpdateAnalysisInformation<br class="">
<br class="">
Summary:<br class="">
We are incorrectly updating the LI when loop-simplify generates<br class="">
dedicated exit blocks for a loop. The issue is that there's an implicit<br class="">
assumption that the Preds passed into UpdateAnalysisInformation are<br class="">
reachable. However, this is not true and breaks LI by incorrectly<br class="">
updating the header of a loop.<br class="">
<br class="">
One such case is when we generate dedicated exits when the exit block is<br class="">
a landing pad (through SplitLandingPadPredecessors). There maybe other<br class="">
cases as well, since we do not guarantee that Preds passed in are<br class="">
reachable basic blocks.<br class="">
<br class="">
The added test case shows how loop-simplify breaks LI for the outer loop (and DT in turn)<br class="">
after we try to generate the LoopSimplifyForm.<br class="">
<br class="">
Reviewers: davide, chandlerc, sanjoy<br class="">
<br class="">
Reviewed By: davide<br class="">
<br class="">
Subscribers: llvm-commits<br class="">
<br class="">
Differential Revision: <a href="https://reviews.llvm.org/D41519" class="">https://reviews.llvm.org/D41519</a><br class="">
<br class="">
Modified:<br class="">
llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp<br class="">
llvm/trunk/test/Transforms/LoopSimplify/unreachable-loop-pred.ll<br class="">
<br class="">
Modified: llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp?rev=321653&r1=321652&r2=321653&view=diff" class="">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp?rev=321653&r1=321652&r2=321653&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp (original)<br class="">
+++ llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp Tue Jan 2 08:25:50 2018<br class="">
@@ -323,6 +323,12 @@ static void UpdateAnalysisInformation(Ba<br class="">
bool IsLoopEntry = !!L;<br class="">
bool SplitMakesNewLoopHeader = false;<br class="">
for (BasicBlock *Pred : Preds) {<br class="">
+ // Preds that are not reachable from entry should not be used to identify if<br class="">
+ // OldBB is a loop entry or if SplitMakesNewLoopHeader. Unreachable blocks<br class="">
+ // are not within any loops, so we incorrectly mark SplitMakesNewLoopHeader<br class="">
+ // as true and make the NewBB the header of some loop. This breaks LI.<br class="">
+ if (!DT->isReachableFromEntry(Pred))<br class="">
</blockquote>
Hi Anna,<br class="">
<br class="">
This change breaks our internal bots because DT might be nullptr as it is optional. Is there another way to check this?<br class="">
</blockquote>
<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">This
code only executes if the LoopInfo pointer is non-null, and LoopInfo requires a DominatorTree to compute, so you probably have a domtree somewhere even if your pass doesn't explicitly require it.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
</div>
</blockquote>
<div>Agree with Eli here. If LI is being passed in to this method, it makes sense to pass in the DT as well (which exists because LI was computed using DT). </div>
<div>Reachability analysis without DT would be from first principles because LI does not contain that information. </div>
<div><br class="">
</div>
<div><br class="">
</div>
<div>Anna</div>
<blockquote type="cite" class="">
<div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">-Eli</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; 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-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Employee
of Qualcomm Innovation Center, Inc.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Qualcomm
Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project</span></div>
</blockquote>
</div>
<br class="">
</body>
</html>