[llvm] b7cd291 - [GlobalOpt] Treat null-check of loaded value as use of global (PR35760)

[Vedant Kumar via llvm-commits]

Author: Vedant Kumar
Date: 2020-03-23T22:36:09-07:00
New Revision: b7cd291c1542aee12c9e9fde6c411314a163a8ea

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

LOG: [GlobalOpt] Treat null-check of loaded value as use of global (PR35760)

PR35760 shows an example program which, when compiled with `clang -O0`
or gcc at any optimization level, prints '0'. However, llvm transforms
the program in a way that causes it to print '1'.

Fix the issue by having `AllUsesOfValueWillTrapIfNull` return false when
analyzing a load from a global which is used by an `icmp`. This special
case was untested [0] so this is just deleting dead code.

An alternative fix might be to change the GlobalStatus analysis for the
global to report "Stored" instead of "StoredOnce". However, "StoredOnce"
is appropriate when only one value other than the initializer is stored
to the global.

[0]
http://lab.llvm.org:8080/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/llvm/lib/Transforms/IPO/GlobalOpt.cpp.html#L662

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

Added: 
    llvm/test/Transforms/GlobalOpt/null-check-is-use-pr35760.ll

Modified: 
    llvm/lib/Transforms/IPO/GlobalOpt.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
index 3b234ca0be7d..0ab6333cb3ac 100644
--- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -659,9 +659,6 @@ static bool AllUsesOfValueWillTrapIfNull(const Value *V,
       // checked.
       if (PHIs.insert(PN).second && !AllUsesOfValueWillTrapIfNull(PN, PHIs))
         return false;
-    } else if (isa<ICmpInst>(U) &&
-               isa<ConstantPointerNull>(U->getOperand(1))) {
-      // Ignore icmp X, null
     } else {
       //cerr << "NONTRAPPING USE: " << *U;
       return false;

diff  --git a/llvm/test/Transforms/GlobalOpt/null-check-is-use-pr35760.ll b/llvm/test/Transforms/GlobalOpt/null-check-is-use-pr35760.ll
new file mode 100644
index 000000000000..32516b6db7ca
--- /dev/null
+++ b/llvm/test/Transforms/GlobalOpt/null-check-is-use-pr35760.ll
@@ -0,0 +1,41 @@
+; RUN: opt -S -globalopt -o - < %s | FileCheck %s
+
+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"
+
+ at _ZL3g_i = internal global i32* null, align 8
+ at .str = private unnamed_addr constant [2 x i8] c"0\00", align 1
+ at .str.1 = private unnamed_addr constant [2 x i8] c"1\00", align 1
+
+define dso_local i32 @main() {
+  store i32* null, i32** @_ZL3g_i, align 8
+  call void @_ZL13PutsSomethingv()
+  ret i32 0
+}
+
+; CHECK-LABEL: define {{.*}} @_ZL13PutsSomethingv()
+; CHECK: [[gvLoad:%.*]] = load i32*, i32** @_ZL3g_i
+; CHECK-NEXT: icmp eq i32* [[gvLoad]], null
+define internal void @_ZL13PutsSomethingv() {
+  %1 = load i32*, i32** @_ZL3g_i, align 8
+  %2 = icmp eq i32* %1, null
+  br i1 %2, label %3, label %7
+
+3:                                                ; preds = %0
+  %4 = call noalias i8* @malloc(i64 4) #3
+  %5 = bitcast i8* %4 to i32*
+  store i32* %5, i32** @_ZL3g_i, align 8
+  %6 = call i32 @puts(i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.str, i64 0, i64 0))
+  br label %9
+
+7:                                                ; preds = %0
+  %8 = call i32 @puts(i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.str.1, i64 0, i64 0))
+  br label %9
+
+9:                                                ; preds = %7, %3
+  ret void
+}
+
+declare dso_local noalias i8* @malloc(i64)
+
+declare dso_local i32 @puts(i8* nocapture readonly)