From 2d9f3520dad7583f4773a301e48b2dd7409b06a9 Mon Sep 17 00:00:00 2001 From: gbaraldi Date: Tue, 25 Feb 2025 10:25:35 -0300 Subject: [PATCH 1/6] Handle vector_insert and vector_extract in late-gc-lowering --- src/llvm-late-gc-lowering.cpp | 83 +++++++++++++++++++++++--------- test/llvmpasses/late-lower-gc.ll | 15 ++++++ 2 files changed, 74 insertions(+), 24 deletions(-) diff --git a/src/llvm-late-gc-lowering.cpp b/src/llvm-late-gc-lowering.cpp index 378466ad36ef1..c37090ddef1b6 100644 --- a/src/llvm-late-gc-lowering.cpp +++ b/src/llvm-late-gc-lowering.cpp @@ -1,6 +1,8 @@ // This file is a part of Julia. License is MIT: https://julialang.org/license #include "llvm-gc-interface-passes.h" +#include "llvm/IR/Intrinsics.h" +#include "llvm/Support/Casting.h" #define DEBUG_TYPE "late_lower_gcroot" @@ -171,39 +173,54 @@ static std::pair FindBaseValue(const State &S, Value *V, bool UseCac (void)LI; break; } - else if (auto II = dyn_cast(CurrentV)) { + else if (dyn_cast(CurrentV) != nullptr && + (cast(CurrentV)->getIntrinsicID() == Intrinsic::masked_load || + cast(CurrentV)->getIntrinsicID() == Intrinsic::masked_gather)) { // Some intrinsics behave like LoadInst followed by a SelectInst // This should never happen in a derived addrspace (since those cannot be stored to memory) // so we don't need to lift these operations, but we do need to check if it's loaded and continue walking the base pointer - if (II->getIntrinsicID() == Intrinsic::masked_load || - II->getIntrinsicID() == Intrinsic::masked_gather) { - if (auto VTy = dyn_cast(II->getType())) { - if (hasLoadedTy(VTy->getElementType())) { - Value *Mask = II->getOperand(2); - Value *Passthrough = II->getOperand(3); - if (!isa(Mask) || !cast(Mask)->isAllOnesValue()) { - assert(isa(Passthrough) && "unimplemented"); - (void)Passthrough; + auto II = dyn_cast(CurrentV); + if (auto VTy = dyn_cast(II->getType())) { + if (hasLoadedTy(VTy->getElementType())) { + Value *Mask = II->getOperand(2); + Value *Passthrough = II->getOperand(3); + if (!isa(Mask) || !cast(Mask)->isAllOnesValue()) { + assert(isa(Passthrough) && "unimplemented"); + (void)Passthrough; + } + CurrentV = II->getOperand(0); + if (II->getIntrinsicID() == Intrinsic::masked_load) { + fld_idx = -1; + if (!isSpecialPtr(CurrentV->getType())) { + CurrentV = ConstantPointerNull::get(PointerType::get(V->getContext(), 0)); } - CurrentV = II->getOperand(0); - if (II->getIntrinsicID() == Intrinsic::masked_load) { - fld_idx = -1; - if (!isSpecialPtr(CurrentV->getType())) { + } else { + if (auto VTy2 = dyn_cast(CurrentV->getType())) { + if (!isSpecialPtr(VTy2->getElementType())) { CurrentV = ConstantPointerNull::get(PointerType::get(V->getContext(), 0)); - } - } else { - if (auto VTy2 = dyn_cast(CurrentV->getType())) { - if (!isSpecialPtr(VTy2->getElementType())) { - CurrentV = ConstantPointerNull::get(PointerType::get(V->getContext(), 0)); - fld_idx = -1; - } + fld_idx = -1; } } - continue; } + continue; + } + } + // In general a load terminates a walk + break; + } + else if (dyn_cast(CurrentV) != nullptr && cast(CurrentV)->getIntrinsicID() == Intrinsic::vector_extract) { + auto II = dyn_cast(CurrentV); + if (auto VTy = dyn_cast(II->getType())) { + if (hasLoadedTy(VTy->getElementType())) { + Value *Idx = II->getOperand(1); + if (!isa(Idx)) { + assert(isa(Idx) && "unimplemented"); + (void)Idx; + } + CurrentV = II->getOperand(0); + fld_idx = -1; + continue; } - // In general a load terminates a walk - break; } } else if (auto CI = dyn_cast(CurrentV)) { @@ -530,6 +547,20 @@ SmallVector LateLowerGCFrame::NumberAllBase(State &S, Value *CurrentV) { Numbers = NumberAll(S, IEI->getOperand(0)); int ElNumber = Number(S, IEI->getOperand(1)); Numbers[idx] = ElNumber; + } else if (dyn_cast(CurrentV) != nullptr && dyn_cast(CurrentV)->getIntrinsicID() == Intrinsic::vector_insert) { + auto *VII = cast(CurrentV); + // Vector insert is a bit like a shuffle so use the same approach + SmallVector Numbers1 = NumberAll(S, VII->getOperand(0)); + SmallVector Numbers2 = NumberAll(S, VII->getOperand(1)); + int first_idx = cast(VII->getOperand(2))->getZExtValue(); + for (unsigned i = 0; i < Numbers1.size(); ++i) { + if (i < first_idx) + Numbers.push_back(Numbers1[i]); + else if (i - first_idx < Numbers2.size()) + Numbers.push_back(Numbers2[i - first_idx]); + else + Numbers.push_back(Numbers1[i]); + } } else if (auto *IVI = dyn_cast(CurrentV)) { Numbers = NumberAll(S, IVI->getAggregateOperand()); auto Tracked = TrackCompositeType(IVI->getType()); @@ -1206,6 +1237,10 @@ State LateLowerGCFrame::LocalScan(Function &F) { } } } + if (II->getIntrinsicID() == Intrinsic::vector_extract || II->getIntrinsicID() == Intrinsic::vector_insert) { + // These are not real defs + continue; + } } auto callee = CI->getCalledFunction(); if (callee && callee == typeof_func) { diff --git a/test/llvmpasses/late-lower-gc.ll b/test/llvmpasses/late-lower-gc.ll index 4739fa186ffc7..af71d3c8d2c75 100644 --- a/test/llvmpasses/late-lower-gc.ll +++ b/test/llvmpasses/late-lower-gc.ll @@ -164,6 +164,21 @@ define {} addrspace(10)* @gclift_switch({} addrspace(13)* addrspace(10)* %input, ret {} addrspace(10)* %ret } +; Shouldn't hang +define void @vector_insert(<4 x {} addrspace(10)* > %0, <2 x {} addrspace(10)* > %1) { +top: + %pgcstack = call {}*** @julia.get_pgcstack() + %2 = call <4 x {} addrspace(10)*> @llvm.vector.insert.v4p10.v2p10(<4 x {} addrspace(10)*> %0, <2 x {} addrspace(10)*> %1, i64 2) + ret void +} + +define void @vector_extract(<4 x {} addrspace(10)* > %0, <2 x {} addrspace(10)* > %1) { +top: + %pgcstack = call {}*** @julia.get_pgcstack() + %2 = call <2 x {} addrspace(10)*> @llvm.vector.extract.v2p10.v4p10(<4 x {} addrspace(10)* > %0, i64 2) + ret void +} + define void @decayar([2 x {} addrspace(10)* addrspace(11)*] %ar) { %v2 = call {}*** @julia.get_pgcstack() %e0 = extractvalue [2 x {} addrspace(10)* addrspace(11)*] %ar, 0 From 01ed95c9d11564b60687b06cc688fa10bf439c71 Mon Sep 17 00:00:00 2001 From: gbaraldi Date: Tue, 25 Feb 2025 10:32:19 -0300 Subject: [PATCH 2/6] Use unsigned instead of int --- src/llvm-late-gc-lowering.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/llvm-late-gc-lowering.cpp b/src/llvm-late-gc-lowering.cpp index c37090ddef1b6..f1903a0d66a0b 100644 --- a/src/llvm-late-gc-lowering.cpp +++ b/src/llvm-late-gc-lowering.cpp @@ -552,7 +552,7 @@ SmallVector LateLowerGCFrame::NumberAllBase(State &S, Value *CurrentV) { // Vector insert is a bit like a shuffle so use the same approach SmallVector Numbers1 = NumberAll(S, VII->getOperand(0)); SmallVector Numbers2 = NumberAll(S, VII->getOperand(1)); - int first_idx = cast(VII->getOperand(2))->getZExtValue(); + unsigned first_idx = cast(VII->getOperand(2))->getZExtValue(); for (unsigned i = 0; i < Numbers1.size(); ++i) { if (i < first_idx) Numbers.push_back(Numbers1[i]); From 359584b86c746cc84a7573c8566d7927f0707028 Mon Sep 17 00:00:00 2001 From: Gabriel Baraldi Date: Tue, 25 Feb 2025 11:22:35 -0300 Subject: [PATCH 3/6] Remove threads from llvmpasses test --- test/llvmpasses/image-codegen.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/llvmpasses/image-codegen.jl b/test/llvmpasses/image-codegen.jl index 35e5add2de601..d594c02a4392e 100644 --- a/test/llvmpasses/image-codegen.jl +++ b/test/llvmpasses/image-codegen.jl @@ -2,7 +2,7 @@ # RUN: export JULIA_LLVM_ARGS="--print-before=loop-vectorize --print-module-scope" # RUN: rm -rf %t # RUN: mkdir %t -# RUN: julia --image-codegen --startup-file=no %s 2> %t/output.txt +# RUN: julia --image-codegen -t1,0 --startup-file=no %s 2> %t/output.txt # RUN: FileCheck %s < %t/output.txt # COM: checks that global variables compiled in imaging codegen From 95c32d9ea8f7e558af0ebbf0d2d47c09e167e81b Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Tue, 11 Mar 2025 13:29:17 +0100 Subject: [PATCH 4/6] only cast to IntrinsicInst once --- src/llvm-late-gc-lowering.cpp | 82 ++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 39 deletions(-) diff --git a/src/llvm-late-gc-lowering.cpp b/src/llvm-late-gc-lowering.cpp index f1903a0d66a0b..ee57d710d3d66 100644 --- a/src/llvm-late-gc-lowering.cpp +++ b/src/llvm-late-gc-lowering.cpp @@ -173,54 +173,57 @@ static std::pair FindBaseValue(const State &S, Value *V, bool UseCac (void)LI; break; } - else if (dyn_cast(CurrentV) != nullptr && - (cast(CurrentV)->getIntrinsicID() == Intrinsic::masked_load || - cast(CurrentV)->getIntrinsicID() == Intrinsic::masked_gather)) { - // Some intrinsics behave like LoadInst followed by a SelectInst - // This should never happen in a derived addrspace (since those cannot be stored to memory) - // so we don't need to lift these operations, but we do need to check if it's loaded and continue walking the base pointer - auto II = dyn_cast(CurrentV); - if (auto VTy = dyn_cast(II->getType())) { - if (hasLoadedTy(VTy->getElementType())) { - Value *Mask = II->getOperand(2); - Value *Passthrough = II->getOperand(3); - if (!isa(Mask) || !cast(Mask)->isAllOnesValue()) { - assert(isa(Passthrough) && "unimplemented"); - (void)Passthrough; - } - CurrentV = II->getOperand(0); - if (II->getIntrinsicID() == Intrinsic::masked_load) { - fld_idx = -1; - if (!isSpecialPtr(CurrentV->getType())) { - CurrentV = ConstantPointerNull::get(PointerType::get(V->getContext(), 0)); + else if (auto II = dyn_cast(CurrentV)) { + if (II->getIntrinsicID() == Intrinsic::masked_load || + II->getIntrinsicID() == Intrinsic::masked_gather) { + // Some intrinsics behave like LoadInst followed by a SelectInst + // This should never happen in a derived addrspace (since those cannot be stored to memory) + // so we don't need to lift these operations, but we do need to check if it's loaded and continue walking the base pointer + if (auto VTy = dyn_cast(II->getType())) { + if (hasLoadedTy(VTy->getElementType())) { + Value *Mask = II->getOperand(2); + Value *Passthrough = II->getOperand(3); + if (!isa(Mask) || !cast(Mask)->isAllOnesValue()) { + assert(isa(Passthrough) && "unimplemented"); + (void)Passthrough; } - } else { - if (auto VTy2 = dyn_cast(CurrentV->getType())) { - if (!isSpecialPtr(VTy2->getElementType())) { + CurrentV = II->getOperand(0); + if (II->getIntrinsicID() == Intrinsic::masked_load) { + fld_idx = -1; + if (!isSpecialPtr(CurrentV->getType())) { CurrentV = ConstantPointerNull::get(PointerType::get(V->getContext(), 0)); - fld_idx = -1; + } + } else { + if (auto VTy2 = dyn_cast(CurrentV->getType())) { + if (!isSpecialPtr(VTy2->getElementType())) { + CurrentV = ConstantPointerNull::get(PointerType::get(V->getContext(), 0)); + fld_idx = -1; + } } } + continue; } - continue; } + // In general a load terminates a walk + break; } - // In general a load terminates a walk - break; - } - else if (dyn_cast(CurrentV) != nullptr && cast(CurrentV)->getIntrinsicID() == Intrinsic::vector_extract) { - auto II = dyn_cast(CurrentV); - if (auto VTy = dyn_cast(II->getType())) { - if (hasLoadedTy(VTy->getElementType())) { - Value *Idx = II->getOperand(1); - if (!isa(Idx)) { - assert(isa(Idx) && "unimplemented"); - (void)Idx; + else if (II->getIntrinsicID() == Intrinsic::vector_extract) { + if (auto VTy = dyn_cast(II->getType())) { + if (hasLoadedTy(VTy->getElementType())) { + Value *Idx = II->getOperand(1); + if (!isa(Idx)) { + assert(isa(Idx) && "unimplemented"); + (void)Idx; + } + CurrentV = II->getOperand(0); + fld_idx = -1; + continue; } - CurrentV = II->getOperand(0); - fld_idx = -1; - continue; } + break; + } else { + // Unknown Intrinsic + break; } } else if (auto CI = dyn_cast(CurrentV)) { @@ -232,6 +235,7 @@ static std::pair FindBaseValue(const State &S, Value *V, bool UseCac break; } else { + // Unknown Instruction break; } } From 05d174c2011986b7b09d65c0ac14292c2cb04e75 Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Tue, 11 Mar 2025 13:52:40 +0100 Subject: [PATCH 5/6] fixup! only cast to IntrinsicInst once --- src/llvm-late-gc-lowering.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/llvm-late-gc-lowering.cpp b/src/llvm-late-gc-lowering.cpp index ee57d710d3d66..bfecce6ed5c98 100644 --- a/src/llvm-late-gc-lowering.cpp +++ b/src/llvm-late-gc-lowering.cpp @@ -173,7 +173,7 @@ static std::pair FindBaseValue(const State &S, Value *V, bool UseCac (void)LI; break; } - else if (auto II = dyn_cast(CurrentV)) { + else if (auto *II = dyn_cast(CurrentV)) { if (II->getIntrinsicID() == Intrinsic::masked_load || II->getIntrinsicID() == Intrinsic::masked_gather) { // Some intrinsics behave like LoadInst followed by a SelectInst @@ -232,6 +232,7 @@ static std::pair FindBaseValue(const State &S, Value *V, bool UseCac CurrentV = CI->getArgOperand(0); continue; } + // Unkown Call break; } else { @@ -551,12 +552,14 @@ SmallVector LateLowerGCFrame::NumberAllBase(State &S, Value *CurrentV) { Numbers = NumberAll(S, IEI->getOperand(0)); int ElNumber = Number(S, IEI->getOperand(1)); Numbers[idx] = ElNumber; - } else if (dyn_cast(CurrentV) != nullptr && dyn_cast(CurrentV)->getIntrinsicID() == Intrinsic::vector_insert) { - auto *VII = cast(CurrentV); + // C++17 + // } else if (auto *II = dyn_cast(CurrentV); II && II->getIntrinsicID() == Intrinsic::vector_insert) { + } else if (isa(CurrentV) && cast(CurrentV)->getIntrinsicID() == Intrinsic::vector_insert) { + auto *II = dyn_cast(CurrentV); // Vector insert is a bit like a shuffle so use the same approach - SmallVector Numbers1 = NumberAll(S, VII->getOperand(0)); - SmallVector Numbers2 = NumberAll(S, VII->getOperand(1)); - unsigned first_idx = cast(VII->getOperand(2))->getZExtValue(); + SmallVector Numbers1 = NumberAll(S, II->getOperand(0)); + SmallVector Numbers2 = NumberAll(S, II->getOperand(1)); + unsigned first_idx = cast(II->getOperand(2))->getZExtValue(); for (unsigned i = 0; i < Numbers1.size(); ++i) { if (i < first_idx) Numbers.push_back(Numbers1[i]); From 06aef186207359521b0536babead67e4a18e2b13 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Wed, 7 May 2025 15:37:56 -0400 Subject: [PATCH 6/6] Update llvm-late-gc-lowering.cpp --- src/llvm-late-gc-lowering.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/llvm-late-gc-lowering.cpp b/src/llvm-late-gc-lowering.cpp index bfecce6ed5c98..4ee7e4b30e61c 100644 --- a/src/llvm-late-gc-lowering.cpp +++ b/src/llvm-late-gc-lowering.cpp @@ -232,7 +232,7 @@ static std::pair FindBaseValue(const State &S, Value *V, bool UseCac CurrentV = CI->getArgOperand(0); continue; } - // Unkown Call + // Unknown Call break; } else {