[llvm] r243040 - [Scalarizer] Fix potential for stale data in Scattered across invocations

[Matt Wala]

Author: wala
Date: Thu Jul 23 15:53:46 2015
New Revision: 243040

URL: http://llvm.org/viewvc/llvm-project?rev=243040&view=rev
Log:
[Scalarizer] Fix potential for stale data in Scattered across invocations

Summary:
Scalarizer has two data structures that hold information about changes
to the function, Gathered and Scattered. These are cleared in finish()
at the end of runOnFunction() if finish() detects any changes to the
function.

However, finish() was checking for changes by only checking if
Gathered was non-empty. The function visitStore() only modifies
Scattered without touching Gathered. As a result, Scattered could have
ended up having stale data if Scalarizer only scalarized store
instructions. Since the data in Scattered is used during the execution
of the pass, this introduced dangling pointer errors.

The fix is to check whether both Scattered and Gathered are empty
before deciding what to do in finish(). This also fixes a problem
where the Function can be modified although the pass returns false.

Reviewers: rnk

Subscribers: rnk, srhines, llvm-commits

Differential Revision: http://reviews.llvm.org/D10459

Added:
    llvm/trunk/test/Transforms/Scalarizer/store-bug.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/Scalarizer.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/Scalarizer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/Scalarizer.cpp?rev=243040&r1=243039&r2=243040&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/Scalarizer.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/Scalarizer.cpp Thu Jul 23 15:53:46 2015
@@ -247,6 +247,7 @@ bool Scalarizer::doInitialization(Module
 }
 
 bool Scalarizer::runOnFunction(Function &F) {
+  assert(Gathered.empty() && Scattered.empty());
   for (Function::iterator BBI = F.begin(), BBE = F.end(); BBI != BBE; ++BBI) {
     BasicBlock *BB = BBI;
     for (BasicBlock::iterator II = BB->begin(), IE = BB->end(); II != IE;) {
@@ -636,7 +637,9 @@ bool Scalarizer::visitStoreInst(StoreIns
 // Delete the instructions that we scalarized.  If a full vector result
 // is still needed, recreate it using InsertElements.
 bool Scalarizer::finish() {
-  if (Gathered.empty())
+  // The presence of data in Gathered or Scattered indicates changes
+  // made to the Function.
+  if (Gathered.empty() && Scattered.empty())
     return false;
   for (GatherList::iterator GMI = Gathered.begin(), GME = Gathered.end();
        GMI != GME; ++GMI) {

Added: llvm/trunk/test/Transforms/Scalarizer/store-bug.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Scalarizer/store-bug.ll?rev=243040&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/Scalarizer/store-bug.ll (added)
+++ llvm/trunk/test/Transforms/Scalarizer/store-bug.ll Thu Jul 23 15:53:46 2015
@@ -0,0 +1,25 @@
+; RUN: opt -scalarizer -scalarize-load-store -S < %s | FileCheck %s
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
+
+; This input caused the scalarizer not to clear cached results
+; properly.
+;
+; Any regressions should trigger an assert in the scalarizer.
+
+define void @func(<4 x float> %val, <4 x float> *%ptr) {
+  store <4 x float> %val, <4 x float> *%ptr
+  ret void
+; CHECK: store float %val.i0, float* %ptr.i0, align 16
+; CHECK: store float %val.i1, float* %ptr.i1, align 4
+; CHECK: store float %val.i2, float* %ptr.i2, align 8
+; CHECK: store float %val.i3, float* %ptr.i3, align 4
+}
+
+define void @func.copy(<4 x float> %val, <4 x float> *%ptr) {
+  store <4 x float> %val, <4 x float> *%ptr
+  ret void
+; CHECK: store float %val.i0, float* %ptr.i0, align 16
+; CHECK: store float %val.i1, float* %ptr.i1, align 4
+; CHECK: store float %val.i2, float* %ptr.i2, align 8
+; CHECK: store float %val.i3, float* %ptr.i3, align 4
+}