[llvm] b58a697 - [LICM] Don't promote store to global even in single-thread mode
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 3 05:20:43 PDT 2023
Author: Nikita Popov
Date: 2023-04-03T14:20:06+02:00
New Revision: b58a697f3e19437f8525ec31a384c8901b0b736c
URL: https://github.com/llvm/llvm-project/commit/b58a697f3e19437f8525ec31a384c8901b0b736c
DIFF: https://github.com/llvm/llvm-project/commit/b58a697f3e19437f8525ec31a384c8901b0b736c.diff
LOG: [LICM] Don't promote store to global even in single-thread mode
Even if there are no thread-safety concerns, we should not promote
(not guaranteed-to-execute) stores to globals without further
analysis: While the global may be writable, we may not have
provenance to perform the write. The @promote_global_noalias test
case illustrates a miscompile in the presence of a noalias pointer
to the global.
Worth noting that the load-only promotion may also not be well-defined
depending on precise semantics (we don't specify whether load
violating noalias is poison or UB -- though I believe the general
inclination is to make it poison, and only stores UB), but that's
a more general issue.
This is inspired by https://github.com/llvm/llvm-project/issues/60860,
which is a related issue with TBAA metadata.
Differential Revision: https://reviews.llvm.org/D146233
Added:
Modified:
llvm/lib/Transforms/Scalar/LICM.cpp
llvm/test/Transforms/LICM/promote-single-thread.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp
index a0db2b38763ff..867b989933e50 100644
--- a/llvm/lib/Transforms/Scalar/LICM.cpp
+++ b/llvm/lib/Transforms/Scalar/LICM.cpp
@@ -1943,6 +1943,8 @@ bool isNotVisibleOnUnwindInLoop(const Value *Object, const Loop *L,
isNotCapturedBeforeOrInLoop(Object, L, DT);
}
+// We don't consider globals as writable: While the physical memory is writable,
+// we may not have provenance to perform the write.
bool isWritableObject(const Value *Object) {
// TODO: Alloca might not be writable after its lifetime ends.
// See https://github.com/llvm/llvm-project/issues/51838.
@@ -1953,9 +1955,6 @@ bool isWritableObject(const Value *Object) {
if (auto *A = dyn_cast<Argument>(Object))
return A->hasByValAttr();
- if (auto *G = dyn_cast<GlobalVariable>(Object))
- return !G->isConstant();
-
// TODO: Noalias has nothing to do with writability, this should check for
// an allocator function.
return isNoAliasCall(Object);
diff --git a/llvm/test/Transforms/LICM/promote-single-thread.ll b/llvm/test/Transforms/LICM/promote-single-thread.ll
index ff0798a877603..89935b5f1ae01 100644
--- a/llvm/test/Transforms/LICM/promote-single-thread.ll
+++ b/llvm/test/Transforms/LICM/promote-single-thread.ll
@@ -7,44 +7,26 @@
declare void @capture(ptr)
-; In single-thread mode both loads and stores can be promoted. In multi-thread
-; mode only loads can be promoted, as a
diff erent thread might write to the
-; global.
+; Even in single-thread mode, can only perform load-only promotion for globals,
+; because we might not have provenance to write to the global. See
+; promote_global_noalias for an example of the issue.
define void @promote_global(i1 %c, i1 %c2) {
-; MT-LABEL: @promote_global(
-; MT-NEXT: entry:
-; MT-NEXT: [[G_PROMOTED:%.*]] = load i32, ptr @g, align 4
-; MT-NEXT: br label [[LOOP:%.*]]
-; MT: loop:
-; MT-NEXT: [[V_INC2:%.*]] = phi i32 [ [[V_INC1:%.*]], [[LATCH:%.*]] ], [ [[G_PROMOTED]], [[ENTRY:%.*]] ]
-; MT-NEXT: br i1 [[C:%.*]], label [[IF:%.*]], label [[LATCH]]
-; MT: if:
-; MT-NEXT: [[V_INC:%.*]] = add i32 [[V_INC2]], 1
-; MT-NEXT: store i32 [[V_INC]], ptr @g, align 4
-; MT-NEXT: br label [[LATCH]]
-; MT: latch:
-; MT-NEXT: [[V_INC1]] = phi i32 [ [[V_INC]], [[IF]] ], [ [[V_INC2]], [[LOOP]] ]
-; MT-NEXT: br i1 [[C2:%.*]], label [[EXIT:%.*]], label [[LOOP]]
-; MT: exit:
-; MT-NEXT: ret void
-;
-; ST-LABEL: @promote_global(
-; ST-NEXT: entry:
-; ST-NEXT: [[G_PROMOTED:%.*]] = load i32, ptr @g, align 4
-; ST-NEXT: br label [[LOOP:%.*]]
-; ST: loop:
-; ST-NEXT: [[V_INC2:%.*]] = phi i32 [ [[V_INC1:%.*]], [[LATCH:%.*]] ], [ [[G_PROMOTED]], [[ENTRY:%.*]] ]
-; ST-NEXT: br i1 [[C:%.*]], label [[IF:%.*]], label [[LATCH]]
-; ST: if:
-; ST-NEXT: [[V_INC:%.*]] = add i32 [[V_INC2]], 1
-; ST-NEXT: br label [[LATCH]]
-; ST: latch:
-; ST-NEXT: [[V_INC1]] = phi i32 [ [[V_INC]], [[IF]] ], [ [[V_INC2]], [[LOOP]] ]
-; ST-NEXT: br i1 [[C2:%.*]], label [[EXIT:%.*]], label [[LOOP]]
-; ST: exit:
-; ST-NEXT: [[V_INC1_LCSSA:%.*]] = phi i32 [ [[V_INC1]], [[LATCH]] ]
-; ST-NEXT: store i32 [[V_INC1_LCSSA]], ptr @g, align 4
-; ST-NEXT: ret void
+; CHECK-LABEL: @promote_global(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[G_PROMOTED:%.*]] = load i32, ptr @g, align 4
+; CHECK-NEXT: br label [[LOOP:%.*]]
+; CHECK: loop:
+; CHECK-NEXT: [[V_INC2:%.*]] = phi i32 [ [[V_INC1:%.*]], [[LATCH:%.*]] ], [ [[G_PROMOTED]], [[ENTRY:%.*]] ]
+; CHECK-NEXT: br i1 [[C:%.*]], label [[IF:%.*]], label [[LATCH]]
+; CHECK: if:
+; CHECK-NEXT: [[V_INC:%.*]] = add i32 [[V_INC2]], 1
+; CHECK-NEXT: store i32 [[V_INC]], ptr @g, align 4
+; CHECK-NEXT: br label [[LATCH]]
+; CHECK: latch:
+; CHECK-NEXT: [[V_INC1]] = phi i32 [ [[V_INC]], [[IF]] ], [ [[V_INC2]], [[LOOP]] ]
+; CHECK-NEXT: br i1 [[C2:%.*]], label [[EXIT:%.*]], label [[LOOP]]
+; CHECK: exit:
+; CHECK-NEXT: ret void
;
entry:
br label %loop
@@ -105,48 +87,26 @@ exit:
; if %c is false and %ptr == @g, then this should store 42 to the pointer.
; However, if we perform load+store promotion, then we would instead store the
; original value of the global.
-; FIXME: This is a miscompile.
define void @promote_global_noalias(i1 %c, i1 %c2, ptr noalias %ptr) {
-; MT-LABEL: @promote_global_noalias(
-; MT-NEXT: entry:
-; MT-NEXT: [[G_PROMOTED:%.*]] = load i32, ptr @g, align 4
-; MT-NEXT: br label [[LOOP:%.*]]
-; MT: loop:
-; MT-NEXT: [[V_INC2:%.*]] = phi i32 [ [[V_INC1:%.*]], [[LATCH:%.*]] ], [ [[G_PROMOTED]], [[ENTRY:%.*]] ]
-; MT-NEXT: br i1 [[C:%.*]], label [[IF:%.*]], label [[ELSE:%.*]]
-; MT: if:
-; MT-NEXT: [[V_INC:%.*]] = add i32 [[V_INC2]], 1
-; MT-NEXT: store i32 [[V_INC]], ptr @g, align 4
-; MT-NEXT: br label [[LATCH]]
-; MT: else:
-; MT-NEXT: store i32 42, ptr [[PTR:%.*]], align 4
-; MT-NEXT: br label [[LATCH]]
-; MT: latch:
-; MT-NEXT: [[V_INC1]] = phi i32 [ [[V_INC2]], [[ELSE]] ], [ [[V_INC]], [[IF]] ]
-; MT-NEXT: br i1 [[C2:%.*]], label [[EXIT:%.*]], label [[LOOP]]
-; MT: exit:
-; MT-NEXT: ret void
-;
-; ST-LABEL: @promote_global_noalias(
-; ST-NEXT: entry:
-; ST-NEXT: [[G_PROMOTED:%.*]] = load i32, ptr @g, align 4
-; ST-NEXT: br label [[LOOP:%.*]]
-; ST: loop:
-; ST-NEXT: [[V_INC2:%.*]] = phi i32 [ [[V_INC1:%.*]], [[LATCH:%.*]] ], [ [[G_PROMOTED]], [[ENTRY:%.*]] ]
-; ST-NEXT: br i1 [[C:%.*]], label [[IF:%.*]], label [[ELSE:%.*]]
-; ST: if:
-; ST-NEXT: [[V_INC:%.*]] = add i32 [[V_INC2]], 1
-; ST-NEXT: br label [[LATCH]]
-; ST: else:
-; ST-NEXT: store i32 42, ptr [[PTR:%.*]], align 4
-; ST-NEXT: br label [[LATCH]]
-; ST: latch:
-; ST-NEXT: [[V_INC1]] = phi i32 [ [[V_INC2]], [[ELSE]] ], [ [[V_INC]], [[IF]] ]
-; ST-NEXT: br i1 [[C2:%.*]], label [[EXIT:%.*]], label [[LOOP]]
-; ST: exit:
-; ST-NEXT: [[V_INC1_LCSSA:%.*]] = phi i32 [ [[V_INC1]], [[LATCH]] ]
-; ST-NEXT: store i32 [[V_INC1_LCSSA]], ptr @g, align 4
-; ST-NEXT: ret void
+; CHECK-LABEL: @promote_global_noalias(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[G_PROMOTED:%.*]] = load i32, ptr @g, align 4
+; CHECK-NEXT: br label [[LOOP:%.*]]
+; CHECK: loop:
+; CHECK-NEXT: [[V_INC2:%.*]] = phi i32 [ [[V_INC1:%.*]], [[LATCH:%.*]] ], [ [[G_PROMOTED]], [[ENTRY:%.*]] ]
+; CHECK-NEXT: br i1 [[C:%.*]], label [[IF:%.*]], label [[ELSE:%.*]]
+; CHECK: if:
+; CHECK-NEXT: [[V_INC:%.*]] = add i32 [[V_INC2]], 1
+; CHECK-NEXT: store i32 [[V_INC]], ptr @g, align 4
+; CHECK-NEXT: br label [[LATCH]]
+; CHECK: else:
+; CHECK-NEXT: store i32 42, ptr [[PTR:%.*]], align 4
+; CHECK-NEXT: br label [[LATCH]]
+; CHECK: latch:
+; CHECK-NEXT: [[V_INC1]] = phi i32 [ [[V_INC2]], [[ELSE]] ], [ [[V_INC]], [[IF]] ]
+; CHECK-NEXT: br i1 [[C2:%.*]], label [[EXIT:%.*]], label [[LOOP]]
+; CHECK: exit:
+; CHECK-NEXT: ret void
;
entry:
br label %loop
More information about the llvm-commits
mailing list