[PATCH] D102711: [GlobalOpt] Handle null check with global pointer variables

[Shimin Cui via Phabricator via llvm-commits]

scui updated this revision to Diff 347965.
scui added a comment.
Herald added a subscriber: jfb.

This to address the review comments: (1) Refine the code to handle ICmpInst when checking if AllUsesOfValueWillTrapIfNull,  (2) Consider the GlobalVariable is initialized when creating StoreInst for the global init bool symbol, (3) Remove the unsafe code sinking when transforming the ICmpInst.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102711/new/

https://reviews.llvm.org/D102711

Files:
  llvm/lib/Transforms/IPO/GlobalOpt.cpp
  llvm/test/Transforms/GlobalOpt/null-check-is-use-pr35760.ll


Index: llvm/test/Transforms/GlobalOpt/null-check-is-use-pr35760.ll
===================================================================
--- llvm/test/Transforms/GlobalOpt/null-check-is-use-pr35760.ll
+++ llvm/test/Transforms/GlobalOpt/null-check-is-use-pr35760.ll
@@ -3,10 +3,13 @@
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
+; CHECK: [[_ZL3G_I_INIT:@.*]] = internal unnamed_addr global i1 false
 @_ZL3g_i = internal global i32* null, align 8
 @.str = private unnamed_addr constant [2 x i8] c"0\00", align 1
 @.str.1 = private unnamed_addr constant [2 x i8] c"1\00", align 1
 
+; CHECK-LABEL: define {{.*}} @main(
+; CHECK-NEXT:    store i1 false, i1* [[_ZL3G_I_INIT]], align 1
 define dso_local i32 @main() {
   store i32* null, i32** @_ZL3g_i, align 8
   call void @_ZL13PutsSomethingv()
@@ -14,8 +17,11 @@
 }
 
 ; CHECK-LABEL: define {{.*}} @_ZL13PutsSomethingv()
-; CHECK: [[gvLoad:%.*]] = load i32*, i32** @_ZL3g_i
-; CHECK-NEXT: icmp eq i32* [[gvLoad]], null
+; CHECK-NEXT:    [[_ZL3G_I_INIT_VAL:%.*]] = load i1, i1* [[_ZL3G_I_INIT]], align 1
+; CHECK-NEXT:    [[NOTINIT:%.*]] = xor i1 [[_ZL3G_I_INIT_VAL]], true
+; CHECK-NEXT:    br i1 [[NOTINIT]], label %[[TMP1:.*]], label %[[TMP3:.*]]
+; CHECK:       [[TMP1]]:
+; CHECK-NEXT:    store i1 true, i1* [[_ZL3G_I_INIT]], align 1
 define internal void @_ZL13PutsSomethingv() {
   %1 = load i32*, i32** @_ZL3g_i, align 8
   %2 = icmp eq i32* %1, null
Index: llvm/lib/Transforms/IPO/GlobalOpt.cpp
===================================================================
--- llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -699,6 +699,14 @@
       // checked.
       if (PHIs.insert(PN).second && !AllUsesOfValueWillTrapIfNull(PN, PHIs))
         return false;
+    } else if (isa<ICmpInst>(U) && isa<LoadInst>(U->getOperand(0)) &&
+               isa<ConstantPointerNull>(U->getOperand(1))) {
+      assert(isa<GlobalValue>(
+                 cast<LoadInst>(U->getOperand(0))->getPointerOperand()) &&
+             "Should be GlobalVariable");
+      // This and only this kind of ICmpInst is to be replaced with the
+      // comparing of the value of the created global init bool later in
+      // optimizeGlobalAddressOfMalloc for the global variable.
     } else {
       //cerr << "NONTRAPPING USE: " << *U;
       return false;
@@ -940,9 +948,13 @@
   // Loop over all uses of GV, processing them in turn.
   while (!GV->use_empty()) {
     if (StoreInst *SI = dyn_cast<StoreInst>(GV->user_back())) {
-      // The global is initialized when the store to it occurs.
-      new StoreInst(ConstantInt::getTrue(GV->getContext()), InitBool, false,
-                    Align(1), SI->getOrdering(), SI->getSyncScopeID(), SI);
+      // The global is initialized when the store to it occurs. If the stored
+      // value is null value, the global bool is set to false, otherwise true.
+      new StoreInst(ConstantInt::getBool(
+                        GV->getContext(),
+                        !isa<ConstantPointerNull>(SI->getValueOperand())),
+                    InitBool, false, Align(1), SI->getOrdering(),
+                    SI->getSyncScopeID(), SI);
       SI->eraseFromParent();
       continue;
     }
@@ -957,11 +969,9 @@
       }
 
       // Replace the cmp X, 0 with a use of the bool value.
-      // Sink the load to where the compare was, if atomic rules allow us to.
       Value *LV = new LoadInst(InitBool->getValueType(), InitBool,
                                InitBool->getName() + ".val", false, Align(1),
-                               LI->getOrdering(), LI->getSyncScopeID(),
-                               LI->isUnordered() ? (Instruction *)ICI : LI);
+                               LI->getOrdering(), LI->getSyncScopeID(), LI);
       InitBoolUsed = true;
       switch (ICI->getPredicate()) {
       default: llvm_unreachable("Unknown ICmp Predicate!");


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D102711.347965.patch
Type: text/x-patch
Size: 3971 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210526/9aa8396b/attachment.bin>