[PATCH] D28940: NewGVN: Fix PR 31686 by rewriting store leader handling.

Davide Italiano via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 20 10:47:09 PST 2017


davide added a comment.

This looks good, thanks for the very quick fix (a minor comment inline)



================
Comment at: lib/Transforms/Scalar/NewGVN.cpp:1224-1235
-  } else if (auto *SI = dyn_cast<StoreInst>(I)) {
-    // There is, sadly, one complicating thing for stores.  Stores do not
-    // produce values, only consume them.  However, in order to make loads and
-    // stores value number the same, we ignore the value operand of the store.
-    // But the value operand will still be the leader of our class, and thus, it
-    // may change.  Because the store is a use, the store will get reprocessed,
-    // but nothing will change about it, and so nothing above will catch it
----------------
Wheee for this going away.


================
Comment at: test/Transforms/NewGVN/pr31686.ll:1-11
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -basicaa -newgvn -S | FileCheck %s
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-scei-ps4"
+
+%struct.ham = type { float, float, float, float, float, float, float, float, float, float, float, float, float, float, float, float }
+%struct.bar = type { i32 (...)** }
----------------
I really think this test can go away as the issue is covered by the other.


https://reviews.llvm.org/D28940





More information about the llvm-commits mailing list