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

[Chandler Carruth]

FYI, this patch went in without proper review and has problems. Please
revert for now, and see my comment on the review thread.

On Fri, Jun 12, 2015 at 3:55 PM Matt Wala <wala at google.com> wrote:

> Author: wala
> Date: Fri Jun 12 17:49:11 2015
> New Revision: 239644
>
> URL: http://llvm.org/viewvc/llvm-project?rev=239644&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().
>
> Reviewers: srhines
>
> Reviewed By: srhines
>
> Subscribers: llvm-commits
>
> Differential Revision: http://reviews.llvm.org/D10422
>
> 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=239644&r1=239643&r2=239644&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/Scalarizer.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/Scalarizer.cpp Fri Jun 12 17:49:11
> 2015
> @@ -636,7 +636,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) {
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150613/5e5038b7/attachment.html>