[llvm] 236fbf5 - [GlobalStatus] Skip non-pointer dead constant users

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2022-02-01T15:51:32+01:00
New Revision: 236fbf571dc6cebcb81ac5187a170c8de6d5bc0e

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

LOG: [GlobalStatus] Skip non-pointer dead constant users

Constant expressions with a non-pointer result type used an early
exit that bypassed the later dead constant user check, and resulted
in different optimization outcomes depending on whether dead users
were present or not.

This fixes the issue reported in https://reviews.llvm.org/D117223#3287039.

Added: 
    llvm/test/Transforms/GlobalOpt/dead-constant-user.ll

Modified: 
    llvm/lib/Transforms/Utils/GlobalStatus.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Utils/GlobalStatus.cpp b/llvm/lib/Transforms/Utils/GlobalStatus.cpp
index f8ec8c6ad4266..c1c5f5cc879f5 100644
--- a/llvm/lib/Transforms/Utils/GlobalStatus.cpp
+++ b/llvm/lib/Transforms/Utils/GlobalStatus.cpp
@@ -65,15 +65,18 @@ static bool analyzeGlobalAux(const Value *V, GlobalStatus &GS,
 
   for (const Use &U : V->uses()) {
     const User *UR = U.getUser();
-    if (const ConstantExpr *CE = dyn_cast<ConstantExpr>(UR)) {
-      // If the result of the constantexpr isn't pointer type, then we won't
-      // know to expect it in various places.  Just reject early.
-      if (!isa<PointerType>(CE->getType()))
-        return true;
-
-      // FIXME: Do we need to add constexpr selects to VisitedUsers?
-      if (analyzeGlobalAux(CE, GS, VisitedUsers))
-        return true;
+    if (const Constant *C = dyn_cast<Constant>(UR)) {
+      const ConstantExpr *CE = dyn_cast<ConstantExpr>(C);
+      if (CE && isa<PointerType>(CE->getType())) {
+        // Recursively analyze pointer-typed constant expressions.
+        // FIXME: Do we need to add constexpr selects to VisitedUsers?
+        if (analyzeGlobalAux(CE, GS, VisitedUsers))
+          return true;
+      } else {
+        // Ignore dead constant users.
+        if (!isSafeToDestroyConstant(C))
+          return true;
+      }
     } else if (const Instruction *I = dyn_cast<Instruction>(UR)) {
       if (!GS.HasMultipleAccessingFunctions) {
         const Function *F = I->getParent()->getParent();
@@ -169,10 +172,6 @@ static bool analyzeGlobalAux(const Value *V, GlobalStatus &GS,
       } else {
         return true; // Any other non-load instruction might take address!
       }
-    } else if (const Constant *C = dyn_cast<Constant>(UR)) {
-      // We might have a dead and dangling constant hanging off of here.
-      if (!isSafeToDestroyConstant(C))
-        return true;
     } else {
       // Otherwise must be some other user.
       return true;

diff  --git a/llvm/test/Transforms/GlobalOpt/dead-constant-user.ll b/llvm/test/Transforms/GlobalOpt/dead-constant-user.ll
new file mode 100644
index 0000000000000..02b2ebe46c09c
--- /dev/null
+++ b/llvm/test/Transforms/GlobalOpt/dead-constant-user.ll
@@ -0,0 +1,23 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -passes='function(early-cse),globalopt' < %s | FileCheck %s
+; RUN: opt -S -passes='function(early-cse)' < %s | opt -S -passes=globalopt | FileCheck %s
+
+ at g = internal global [6 x i16*] undef
+
+define void @test1() {
+; CHECK-LABEL: @test1(
+; CHECK-NEXT:    ret void
+;
+  %xor4 = xor i32 zext (i1 icmp ne (i8* getelementptr (i8, i8* bitcast ([6 x i16*]* @g to i8*), i64 3), i8* null) to i32), 0
+  %t0 = load i16*, i16** bitcast (i8* getelementptr (i8, i8* bitcast ([6 x i16*]* @g to i8*), i64 3) to i16**), align 1
+  %t1 = load i16, i16* %t0, align 1
+  ret void
+}
+
+define void @test2() {
+; CHECK-LABEL: @test2(
+; CHECK-NEXT:    ret void
+;
+  store i16* null, i16** getelementptr inbounds ([6 x i16*], [6 x i16*]* @g, i32 0, i32 5)
+  ret void
+}