From 7b6646f68bcaca5fa4f3f52de63f23f6960f97e0 Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Thu, 13 Feb 2025 12:03:23 +0100 Subject: [PATCH 1/3] Only strip invariant.load from special pointers Other backends (in this case NVPTX) require that `invariant.load` metadata is maintained to generate non-coherent loads. Currently, we unconditionally strip that metadata from all loads, since our other uses of it may have become invalid. --- src/llvm-late-gc-lowering.cpp | 6 ++++-- test/llvmpasses/late-lower-gc.ll | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/llvm-late-gc-lowering.cpp b/src/llvm-late-gc-lowering.cpp index 7d6fba65a79e7..2d01aa6cddab0 100644 --- a/src/llvm-late-gc-lowering.cpp +++ b/src/llvm-late-gc-lowering.cpp @@ -1979,8 +1979,10 @@ bool LateLowerGCFrame::CleanupIR(Function &F, State *S, bool *CFGModified) { // strip all constant alias information, as it might depend on the gc having // preserved a gc root, which stops being true after this pass (#32215) // similar to RewriteStatepointsForGC::stripNonValidData, but less aggressive - if (I->getMetadata(LLVMContext::MD_invariant_load)) - I->setMetadata(LLVMContext::MD_invariant_load, NULL); + if (auto *LI = dyn_cast(I)){ + if (isSpecialPtr(LI->getPointerOperand()->getType()) && LI->getMetadata(LLVMContext::MD_invariant_load)) + LI->setMetadata(LLVMContext::MD_invariant_load, NULL); + } if (MDNode *TBAA = I->getMetadata(LLVMContext::MD_tbaa)) { if (TBAA->getNumOperands() == 4 && isTBAA(TBAA, {"jtbaa_const", "jtbaa_memoryptr", "jtbaa_memorylen", "tbaa_memoryown"})) { MDNode *MutableTBAA = createMutableTBAAAccessTag(TBAA); diff --git a/test/llvmpasses/late-lower-gc.ll b/test/llvmpasses/late-lower-gc.ll index d294847db8f9d..1d5f62d2994e1 100644 --- a/test/llvmpasses/late-lower-gc.ll +++ b/test/llvmpasses/late-lower-gc.ll @@ -90,6 +90,22 @@ top: ret void } +; Confirm that `invariant.load` on other loads survive +define void @gc_keep_invariant(float addrspace(1)* %0) { +top: +; CHECK-LABEL: @gc_drop_aliasing + %pgcstack = call {}*** @julia.get_pgcstack() + %1 = bitcast {}*** %pgcstack to {}** + %current_task = getelementptr inbounds {}*, {}** %0, i64 -12 + +; CHECK: %current_task = getelementptr inbounds ptr, ptr %0, i64 -12 +; CHECK-NEXT: [[ptls_field:%.*]] = getelementptr inbounds i8, ptr %current_task, +; CHECK-NEXT: [[ptls_load:%.*]] = load ptr, ptr [[ptls_field]], align 8, !tbaa !0 + %2 = load float, float addrspace(1)* %0, align 4, !invariant.load +; CHECK-NEXT: = load float, float addrspace(1)* %0, align 4, !invariant.load + ret void +} + define i32 @callee_root({} addrspace(10)* %v0, {} addrspace(10)* %v1) { top: ; CHECK-LABEL: @callee_root From a679390d0f9fce24f4b84ea732110b6b1373e2ab Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Fri, 14 Feb 2025 00:55:34 +0100 Subject: [PATCH 2/3] Update test/llvmpasses/late-lower-gc.ll Co-authored-by: Gabriel Baraldi --- test/llvmpasses/late-lower-gc.ll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/llvmpasses/late-lower-gc.ll b/test/llvmpasses/late-lower-gc.ll index 1d5f62d2994e1..77855fe5fa1f3 100644 --- a/test/llvmpasses/late-lower-gc.ll +++ b/test/llvmpasses/late-lower-gc.ll @@ -93,7 +93,7 @@ top: ; Confirm that `invariant.load` on other loads survive define void @gc_keep_invariant(float addrspace(1)* %0) { top: -; CHECK-LABEL: @gc_drop_aliasing +; CHECK-LABEL: @gc_keep_invariant %pgcstack = call {}*** @julia.get_pgcstack() %1 = bitcast {}*** %pgcstack to {}** %current_task = getelementptr inbounds {}*, {}** %0, i64 -12 From 25aeb4694035f9dd89abff8e964346a804c667ff Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Fri, 14 Feb 2025 16:21:13 +0100 Subject: [PATCH 3/3] fixup! Update test/llvmpasses/late-lower-gc.ll --- src/llvm-late-gc-lowering.cpp | 2 +- test/llvmpasses/late-lower-gc.ll | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/llvm-late-gc-lowering.cpp b/src/llvm-late-gc-lowering.cpp index 2d01aa6cddab0..46cc38839500e 100644 --- a/src/llvm-late-gc-lowering.cpp +++ b/src/llvm-late-gc-lowering.cpp @@ -1979,7 +1979,7 @@ bool LateLowerGCFrame::CleanupIR(Function &F, State *S, bool *CFGModified) { // strip all constant alias information, as it might depend on the gc having // preserved a gc root, which stops being true after this pass (#32215) // similar to RewriteStatepointsForGC::stripNonValidData, but less aggressive - if (auto *LI = dyn_cast(I)){ + if (auto *LI = dyn_cast(I)){ if (isSpecialPtr(LI->getPointerOperand()->getType()) && LI->getMetadata(LLVMContext::MD_invariant_load)) LI->setMetadata(LLVMContext::MD_invariant_load, NULL); } diff --git a/test/llvmpasses/late-lower-gc.ll b/test/llvmpasses/late-lower-gc.ll index 77855fe5fa1f3..81a1df61d3bd9 100644 --- a/test/llvmpasses/late-lower-gc.ll +++ b/test/llvmpasses/late-lower-gc.ll @@ -96,13 +96,11 @@ top: ; CHECK-LABEL: @gc_keep_invariant %pgcstack = call {}*** @julia.get_pgcstack() %1 = bitcast {}*** %pgcstack to {}** - %current_task = getelementptr inbounds {}*, {}** %0, i64 -12 + %current_task = getelementptr inbounds {}*, {}** %1, i64 -12 -; CHECK: %current_task = getelementptr inbounds ptr, ptr %0, i64 -12 -; CHECK-NEXT: [[ptls_field:%.*]] = getelementptr inbounds i8, ptr %current_task, -; CHECK-NEXT: [[ptls_load:%.*]] = load ptr, ptr [[ptls_field]], align 8, !tbaa !0 - %2 = load float, float addrspace(1)* %0, align 4, !invariant.load -; CHECK-NEXT: = load float, float addrspace(1)* %0, align 4, !invariant.load +; CHECK: %current_task = getelementptr inbounds ptr, ptr %1, i64 -12 + %2 = load float, ptr addrspace(1) %0, align 4, !invariant.load !1 +; CHECK-NEXT: %2 = load float, ptr addrspace(1) %0, align 4, !invariant.load ret void }