[PATCH] Remove dangling BlockAddresses in GlobalDCE

[Bruno Cardoso Lopes]

Hi Rafael,

I've tried this path before and the problem I found is exactly what you fix with:
 
+  if (isa<ConstantInt>(C) || isa<ConstantFP>(C)) // why?
+    return false;
+

I->setInitializer(nullptr) will actually destroy the constant in ConstantInt and ConstantFP and later on there's nothing there for destroyConstant to destroy.

This is not the case for other underlying constant types - which you can safely keep track by saving the initializer and deleting it later. I prefer your approach, but I suggest we do not change isSafeToDestroyConstant, but check this condition before. See the attached patch.

http://reviews.llvm.org/D4932

Files:
  lib/Transforms/IPO/GlobalDCE.cpp
  test/Transforms/GlobalDCE/deadblockaddr.ll

Index: lib/Transforms/IPO/GlobalDCE.cpp
===================================================================
--- lib/Transforms/IPO/GlobalDCE.cpp
+++ lib/Transforms/IPO/GlobalDCE.cpp
@@ -22,6 +22,7 @@
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Transforms/Utils/CtorUtils.h"
+#include "llvm/Transforms/Utils/GlobalStatus.h"
 #include "llvm/Pass.h"
 using namespace llvm;
 
@@ -141,7 +142,16 @@
        I != E; ++I)
     if (!AliveGlobals.count(I)) {
       DeadGlobalVars.push_back(I);         // Keep track of dead globals
-      I->setInitializer(nullptr);
+      if (I->hasInitializer()) {
+        Constant *Init = I->getInitializer();
+        I->setInitializer(nullptr);
+
+        // Setting initializers to nullptr is enough for Int and FP but not
+        // for other underlying constant types
+        bool IsIntOrFPCst = isa<ConstantInt>(Init) || isa<ConstantFP>(Init);
+        if (!IsIntOrFPCst && isSafeToDestroyConstant(Init))
+          Init->destroyConstant();
+      }
     }
 
   // The second pass drops the bodies of functions which are dead...
Index: test/Transforms/GlobalDCE/deadblockaddr.ll
===================================================================
--- /dev/null
+++ test/Transforms/GlobalDCE/deadblockaddr.ll
@@ -0,0 +1,16 @@
+; RUN: opt -globaldce -simplifycfg -S < %s | FileCheck %s
+
+; Tests whether globaldce does the right cleanup while removing @bar
+; so that a dead BlockAddress reference to foo won't prevent other passes
+; to work properly, e.g. simplifycfg
+ at bar = internal unnamed_addr constant i8* blockaddress(@foo, %L1)
+
+; CHECK-LABEL: foo
+; CHECK-NOT: br label %L1
+; CHECK: ret void
+define void @foo() {
+entry:
+  br label %L1
+L1:
+  ret void
+}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D4932.12854.patch
Type: text/x-patch
Size: 1743 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140822/48cbfd71/attachment.bin>