[GlobalOpt] Do not promote malloc if there are atomic loads/stores

[nikic]

When converting a malloc stored to a global into a global, we will introduce an i1 flag to track whether the global has been initialized.

In case of atomic loads/stores, this will result in verifier failures, because atomic ops on i1 are illegal. Even if we changed this to i8, I don't think it is a good idea to change atomic types in that way.

Instead, bail out of the transform is we encounter any atomic loads/stores of the global.

Fixes #137152.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Nikita Popov (nikic)

Changes

When converting a malloc stored to a global into a global, we will introduce an i1 flag to track whether the global has been initialized.

In case of atomic loads/stores, this will result in verifier failures, because atomic ops on i1 are illegal. Even if we changed this to i8, I don't think it is a good idea to change atomic types in that way.

Instead, bail out of the transform is we encounter any atomic loads/stores of the global.

Fixes #137152.

---

Full diff: https://github.com/llvm/llvm-project/pull/137158.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/IPO/GlobalOpt.cpp (+4)
• (added) llvm/test/Transforms/GlobalOpt/malloc-promote-atomic.ll (+28)

diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
index 83cc1e5f04f3d..1424c4abdb3a6 100644
--- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -719,10 +719,14 @@ static bool allUsesOfLoadedValueWillTrapIfNull(const GlobalVariable *GV) {
     const Value *P = Worklist.pop_back_val();
     for (const auto *U : P->users()) {
       if (auto *LI = dyn_cast<LoadInst>(U)) {
+        if (!LI->isSimple())
+          return false;
         SmallPtrSet<const PHINode *, 8> PHIs;
         if (!AllUsesOfValueWillTrapIfNull(LI, PHIs))
           return false;
       } else if (auto *SI = dyn_cast<StoreInst>(U)) {
+        if (!SI->isSimple())
+          return false;
         // Ignore stores to the global.
         if (SI->getPointerOperand() != P)
           return false;
diff --git a/llvm/test/Transforms/GlobalOpt/malloc-promote-atomic.ll b/llvm/test/Transforms/GlobalOpt/malloc-promote-atomic.ll
new file mode 100644
index 0000000000000..0ecdf095efdd8
--- /dev/null
+++ b/llvm/test/Transforms/GlobalOpt/malloc-promote-atomic.ll
@@ -0,0 +1,28 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -passes=globalopt -S < %s | FileCheck %s
+
+@g = internal global ptr null, align 8
+
+define void @init() {
+; CHECK-LABEL: define void @init() local_unnamed_addr {
+; CHECK-NEXT:    [[ALLOC:%.*]] = call ptr @malloc(i64 48)
+; CHECK-NEXT:    store atomic ptr [[ALLOC]], ptr @g seq_cst, align 8
+; CHECK-NEXT:    ret void
+;
+  %alloc = call ptr @malloc(i64 48)
+  store atomic ptr %alloc, ptr @g seq_cst, align 8
+  ret void
+}
+
+define i1 @check() {
+; CHECK-LABEL: define i1 @check() local_unnamed_addr {
+; CHECK-NEXT:    [[VAL:%.*]] = load atomic ptr, ptr @g seq_cst, align 8
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq ptr [[VAL]], null
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %val = load atomic ptr, ptr @g seq_cst, align 8
+  %cmp = icmp eq ptr %val, null
+  ret i1 %cmp
+}
+
+declare ptr @malloc(i64) allockind("alloc,uninitialized") allocsize(0)

[dtcxzyw]

Make sense to me.

[fhahn]

LGTM, thanks!