[llvm] fce44ff - [Scalarizer] Avoid updating the name of globals

[Bjorn Pettersson via llvm-commits]

Author: Bjorn Pettersson
Date: 2020-08-24T21:55:03+02:00
New Revision: fce44ff5da4eba4b781d73d72a18903215962091

URL: https://github.com/llvm/llvm-project/commit/fce44ff5da4eba4b781d73d72a18903215962091
DIFF: https://github.com/llvm/llvm-project/commit/fce44ff5da4eba4b781d73d72a18903215962091.diff

LOG: [Scalarizer] Avoid updating the name of globals

The "takeName" logic at the end of ScalarizerVisitor::finish
could end up renaming global variables when having simplified
and extractelement instruction to simply pick a single vector
element. If the input vector to the extractelement instruction
held pointers to global variables we ended up renaming the global
variable.
The patch make sure we only take the name of the replaced Op when
we have added new instructions that might need a useful name.

Reviewed By: lebedev.ri

Differential Revision: https://reviews.llvm.org/D86472

Added: 
    llvm/test/Transforms/Scalarizer/global-bug.ll

Modified: 
    llvm/lib/Transforms/Scalar/Scalarizer.cpp
    llvm/test/Transforms/Scalarizer/basic.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/Scalarizer.cpp b/llvm/lib/Transforms/Scalar/Scalarizer.cpp
index 851bd79cd6d8..3bc0cbde8c19 100644
--- a/llvm/lib/Transforms/Scalar/Scalarizer.cpp
+++ b/llvm/lib/Transforms/Scalar/Scalarizer.cpp
@@ -941,13 +941,13 @@ bool ScalarizerVisitor::finish() {
         for (unsigned I = 0; I < Count; ++I)
           Res = Builder.CreateInsertElement(Res, CV[I], Builder.getInt32(I),
                                             Op->getName() + ".upto" + Twine(I));
+        Res->takeName(Op);
       } else {
         assert(CV.size() == 1 && Op->getType() == CV[0]->getType());
         Res = CV[0];
         if (Op == Res)
           continue;
       }
-      Res->takeName(Op);
       Op->replaceAllUsesWith(Res);
     }
     PotentiallyDeadInstrs.emplace_back(Op);

diff  --git a/llvm/test/Transforms/Scalarizer/basic.ll b/llvm/test/Transforms/Scalarizer/basic.ll
index 239cdd660a33..c8295346b206 100644
--- a/llvm/test/Transforms/Scalarizer/basic.ll
+++ b/llvm/test/Transforms/Scalarizer/basic.ll
@@ -542,8 +542,8 @@ define <2 x float> @f22(<2 x float> %x, <2 x float> %y, <2 x float> %z) {
 ; See https://reviews.llvm.org/D83101#2133062
 define <2 x i32> @f23_crash(<2 x i32> %srcvec, i32 %v1) {
 ; CHECK-LABEL: @f23_crash(
-; CHECK: %1 = extractelement <2 x i32> %srcvec, i32 0
-; CHECK: %t1.upto0 = insertelement <2 x i32> undef, i32 %1, i32 0
+; CHECK: %v0 = extractelement <2 x i32> %srcvec, i32 0
+; CHECK: %t1.upto0 = insertelement <2 x i32> undef, i32 %v0, i32 0
 ; CHECK: %t1 = insertelement <2 x i32> %t1.upto0, i32 %v1, i32 1
 ; CHECK: ret <2 x i32> %t1
   %v0 = extractelement <2 x i32> %srcvec, i32 0

diff  --git a/llvm/test/Transforms/Scalarizer/global-bug.ll b/llvm/test/Transforms/Scalarizer/global-bug.ll
new file mode 100644
index 000000000000..37a01844a359
--- /dev/null
+++ b/llvm/test/Transforms/Scalarizer/global-bug.ll
@@ -0,0 +1,23 @@
+; RUN: opt < %s -scalarizer -S -o - | FileCheck %s
+; RUN: opt < %s -passes='function(scalarizer)' -S | FileCheck %s
+
+ at a = dso_local global i16 0, align 1
+ at b = dso_local local_unnamed_addr global i16 0, align 1
+
+; The scalarizer used to take the name of the extractelement instruction
+; ("extract") and put that on the extracted value, which in this test case is
+; the global variable @a. That was wrong, as we must not change the name of
+; the global variable. So make sure we find "@a" in the ptrtoint.
+define dso_local void @test1() local_unnamed_addr {
+; CHECK-LABEL: @test1(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[T0:%.*]] = ptrtoint i16* @a to i16
+; CHECK-NEXT:    store i16 [[T0]], i16* @b, align 1
+; CHECK-NEXT:    ret void
+;
+entry:
+  %extract = extractelement <4 x i16*> <i16* @a, i16* @a, i16* @a, i16* @a>, i32 1
+  %t0 = ptrtoint i16* %extract to i16
+  store i16 %t0, i16* @b, align 1
+  ret void
+}