<div dir="ltr">FYI, this patch went in without proper review and has problems. Please revert for now, and see my comment on the review thread.<br></div><br><div class="gmail_quote"><div dir="ltr">On Fri, Jun 12, 2015 at 3:55 PM Matt Wala <<a href="mailto:wala@google.com">wala@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: wala<br>
Date: Fri Jun 12 17:49:11 2015<br>
New Revision: 239644<br>
<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D239644-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=XqhTpYUcf45234iWiUMhu9-Mxgb-vV4PcMRnYDfaEIA&s=uTRAS3Tnz9Q4fiTTig5-VbLdgsSp36dIvQp9a_9O9_0&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=239644&view=rev</a><br>
Log:<br>
[Scalarizer] Fix potential for stale data in Scattered across invocations<br>
<br>
Summary:<br>
Scalarizer has two data structures that hold information about changes<br>
to the function, Gathered and Scattered. These are cleared in finish()<br>
at the end of runOnFunction() if finish() detects any changes to the<br>
function.<br>
<br>
However, finish() was checking for changes by only checking if<br>
Gathered was non-empty. The function visitStore() only modifies<br>
Scattered without touching Gathered. As a result, Scattered could have<br>
ended up having stale data if Scalarizer only scalarized store<br>
instructions. Since the data in Scattered is used during the execution<br>
of the pass, this introduced dangling pointer errors.<br>
<br>
The fix is to check whether both Scattered and Gathered are empty<br>
before deciding what to do in finish().<br>
<br>
Reviewers: srhines<br>
<br>
Reviewed By: srhines<br>
<br>
Subscribers: llvm-commits<br>
<br>
Differential Revision: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10422&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=XqhTpYUcf45234iWiUMhu9-Mxgb-vV4PcMRnYDfaEIA&s=2VKx9s4wkQALzqeR_Ua74YXRUp7XlnCQW6dAS4cxSwo&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D10422</a><br>
<br>
Modified:<br>
    llvm/trunk/lib/Transforms/Scalar/Scalarizer.cpp<br>
<br>
Modified: llvm/trunk/lib/Transforms/Scalar/Scalarizer.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Transforms_Scalar_Scalarizer.cpp-3Frev-3D239644-26r1-3D239643-26r2-3D239644-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=XqhTpYUcf45234iWiUMhu9-Mxgb-vV4PcMRnYDfaEIA&s=zLspxwkN_PQXNFGYsu-45rv8ystupqBoZiKQZaL1siw&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/Scalarizer.cpp?rev=239644&r1=239643&r2=239644&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Scalar/Scalarizer.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Scalar/Scalarizer.cpp Fri Jun 12 17:49:11 2015<br>
@@ -636,7 +636,9 @@ bool Scalarizer::visitStoreInst(StoreIns<br>
 // Delete the instructions that we scalarized.  If a full vector result<br>
 // is still needed, recreate it using InsertElements.<br>
 bool Scalarizer::finish() {<br>
-  if (Gathered.empty())<br>
+  // The presence of data in Gathered or Scattered indicates changes<br>
+  // made to the Function.<br>
+  if (Gathered.empty() && Scattered.empty())<br>
     return false;<br>
   for (GatherList::iterator GMI = Gathered.begin(), GME = Gathered.end();<br>
        GMI != GME; ++GMI) {<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>