Add no-op uses to anchor alloca on Wasm#5174
Conversation
Thanks for the extensive explanation and rationale. The whole text should be added to the code as documentation, so it won't get lost! |
Perhaps emit a warning when WASM target and LTO are specified together? |
Oh, that's a good point. I hadn't though of that... If LTO can strip those calls away successfully (since it's a empty one) then I don't need this. I'll see if I can maybe disable on LTO (and it will still do away with the calls). |
c75d990 to
ffe3d3f
Compare
|
Thx for the work! This approach is very intrusive (not nicely isolated for wasm only); I guess there'll be LLVM additions in the future to make this easier, for all other languages with builtin GC. Some refs I've found after a quick search:
An LLVM pass like the last one might be a way forward for now if you don't wanna wait. We could presumably attach IR metadata to IR types, to store the has-potential-GC-pointers flag (for wasm only). |
Yeah, I've seen those. But they are pretty far off right now. Stack switching is closer, but that won't let us read. And But your idea is an interesting one. I had considered something to that effect, but couldn't figure out the union situation. But that just might be the solution (and a less error prone one at that). It might take two passes. One that takes those unions (due to the opaqueness of their type; some metadata could work, but it has to be acted upon early in the pipeline before it is lost) and forces them to stay on the stack like what I did here. Then a later pass can take IR-level |
But you won't control when LTO's "opt" is removing such calls; it might do more optimization afterward. Just to be clear, the case I mean is:
It is almost exactly this case, where bc is outputted into an object file instead of textual IR. |
Yeah, I understood that. And what I'm saying is if we know we passed But it might not matter if I completely change the approach here... |
LTO could remove the call, and then do more optimzation, no?
:-) |
... -_- Right...well that's not going to work then... |
|
Ok, I figured out how to work around that issue.
Since it's inline assembly, it can't be reasoned about by LLVM. The Unfortunately, after some thought, I don't think we can rely on metadata attached to the type for this. It'd probably work for unions, but |
QuantumSegfault
changed the title
_d_stack_gcroot to anchor alloca on Wasmalloca on Wasm
Nope. Doesn't look like you can attach IR metadata to types. Only global declarations ( So the metadata would have to be attached to the |
|
If this is too invasive...I can't really think of much else (that isn't either harmfully conservative or too imprecise/leaky). We need some way to know if there are hidden pointers in |
|
Instead of tackling this at the DtoAlloca* helpers level, I'd suggest handling the param and local allocas in |
|
And wrt. to the inline-asm dummy-use: I've just seen https://llvm.org/docs/LangRef.html#llvm-fake-use-intrinsic, that might do it as well. |
But won't we then miss alloca's that happen in other places? It's an intrusive change, but only because DtoAlloca* does not know the D-type of what is being allocated; if the type would already be known inside DtoAlloca*, the change would be very small. Would it not be best to first create that situation of simply adding the d-type to DtoAlloca* function signature (which may come in handy for other things in the future). And then as 2nd step add the Wasm bit inside DtoAlloca*? |
|
We use helper allocas in various places, like the ABI shenanigans for reinterpret-casts etc. There is no need to handle all those allocas. All we need to care about are params and locals (incl. temporaries), and keeping them on the stack for wasm. Those can be taken care of with presumably a few lines in the 2 functions I've mentioned. Then there are probably a few more allocas, like non-GC closure frames, the implicit I just don't wanna have that flag invade all Dto[Raw]Alloca* usages, because of an exotic target (edit: or rather, an exotic feature - GC - of that target, which surely requires other work to get it somewhat working) and its (current) limitations. I doubt we'll have such an alloca-'pinning' method still in 5 years. |
Okay...I see what you are getting at. Thanks for the explanation. An easy first step I suppose would be to just lift inserting the pinning to each call-site. Then whittle it down from there. |
|
FWIW, I was thinking of starting with something like this: diff --git a/gen/functions.cpp b/gen/functions.cpp
index 023c264b1a..f978fbd491 100644
--- a/gen/functions.cpp
+++ b/gen/functions.cpp
@@ -841,6 +841,10 @@ void defineParameters(IrFuncTy &irFty, VarDeclarations ¶meters) {
// Let the ABI transform the parameter back to an lvalue.
irparam->value =
irFty.getParamLVal(paramType, llArgIdx, irparam->value);
+
+ if (auto alloca = dyn_cast<AllocaInst>(irparam->value)) {
+ pinAllocaForWasm(alloca, paramType);
+ }
}
irparam->value->setName(vd->ident->toChars());
diff --git a/gen/llvmhelpers.cpp b/gen/llvmhelpers.cpp
index 655cb318b9..af886cc490 100644
--- a/gen/llvmhelpers.cpp
+++ b/gen/llvmhelpers.cpp
@@ -924,6 +924,8 @@ void DtoVarDeclaration(VarDeclaration *vd) {
// point it is declared
gIR->funcGen().localVariableLifetimeAnnotator.addLocalVariable(
allocainst, DtoConstUlong(size(type)));
+
+ pinAllocaForWasm(allocainst, type);
}
}
This should handle normal params and locals. |
|
Okay, I've stripped it back down and refactored it. I haven't tested it yet. I'm pretty sure there are going to be some more pins needed. But it does seem cleaner so far. |
|
Ah...we actually have a bigger problem that brings this whole idea down. void* retPtr();
void takePtrs(void*,void*);
void test() {
takePtrs(retPtr(), retPtr()+4);
}Unoptimized: define void @_D9test_libc4testFZv() #0 {
%1 = call ptr @_D9test_libc6retPtrFZPv() #1 ; [#uses = 1]
%2 = call ptr @_D9test_libc6retPtrFZPv() #1 ; [#uses = 1]
%3 = getelementptr inbounds i8, ptr %2, i32 4 ; [#uses = 1, type = ptr]
call void @_D9test_libc8takePtrsFPvQcZv(ptr %1, ptr %3) #1
ret void
}The I need to think about this more... |
Yep, looks much better to me!
Right :/ - we only get temporaries (from the frontend) if the type is a non-POD AFAIK, not for pointers/class refs. [Once we are in the |
| } else if (newexp->onstack) { | ||
| mem = DtoRawAlloca(irClass->getLLStructType(), tc->sym->alignsize, | ||
| ".newclass_alloca"); | ||
| InsertWasmAllocaPin(mem, tc); |
There was a problem hiding this comment.
tc represents a class ref, which is always treated as a potential GC pointer itself. We'd need to query whether the class instance might contain GC pointers in some field, analogous to the has-pointers TypeInfo flag:
Lines 327 to 334 in 2b47d40
| // Let the ABI transform the parameter back to an lvalue. | ||
| irparam->value = | ||
| irFty.getParamLVal(paramType, llArgIdx, irparam->value); | ||
| InsertWasmAllocaPin(irparam->value, paramType); |
There was a problem hiding this comment.
[I don't think we are guaranteed to always get an alloca here, just wrt. the llvm::isa<llvm::AllocaInst> assertion. Not that a dummy-use of a non-alloca would be problematic, just the assertion might be.]
|
Perhaps something like this would be preferred: 4069999 A spill pass to happen after all optimizations. It works at the moment, but is extremely conservative (any vreg that crosses a call triggers it). With some work, it can be refined to only attack potential pointers. If done right, it has the opportunity to be more precise (less pezzimizing) than forcing entire |
I think the most important thing is to get GC working correctly first, then start optimizing/improving from there. |
Fair, but after working on #5178, I've realized that this approach doesn't work as is. At least, not without creating more Take this simple example: void usePtr(void*);
void* getPtr();
size_t maybeTriggetGCForWhateverReason();
void test() {
usePtr(cast(void*)(cast(size_t)getPtr() & maybeTriggetGCForWhateverReason()));
}Even at -O0, we have: define void @_D9test_libc4testFZv() #0 {
%1 = call ptr @_D9test_libc6getPtrFZPv() #1 ; [#uses = 1]
%2 = ptrtoint ptr %1 to i32 ; [#uses = 2]
%3 = call i32 @_D9test_libc31maybeTriggetGCForWhateverReasonFZk() #1 ; [#uses = 1]
%4 = and i32 %2, %3 ; [#uses = 1]
%5 = inttoptr i32 %4 to ptr ; [#uses = 2]
call void @_D9test_libc6usePtrFPvZv(ptr %5) #1
ret void
}Even if we were to say, dump all parameters to alloca, we'd only being storing Vs. with my other PR: No issues. And for now, it errs on the side of caution for the most part: define void @_D9test_libc4testFZv() local_unnamed_addr #0 {
%stackSpill.unnamedVreg = alloca ptr, align 4 ; [#uses = 1, size/byte = 4]
%stackSpill.unnamedVreg1 = alloca i32, align 4 ; [#uses = 1, size/byte = 4]
%stackSpill.unnamedVreg2 = alloca i32, align 4 ; [#uses = 1, size/byte = 4]
%1 = tail call ptr @_D9test_libc6getPtrFZPv() #1 ; [#uses = 1]
%2 = ptrtoint ptr %1 to i32 ; [#uses = 2]
store volatile i32 %2, ptr %stackSpill.unnamedVreg1, align 4
%3 = tail call i32 @_D9test_libc31maybeTriggetGCForWhateverReasonFZk() #1 ; [#uses = 2]
store volatile i32 %3, ptr %stackSpill.unnamedVreg2, align 4
%4 = tail call i32 @_D9test_libc31maybeTriggetGCForWhateverReasonFZk() #1 ; [#uses = 1]
%5 = add i32 %4, %3 ; [#uses = 1]
%6 = and i32 %5, %2 ; [#uses = 1]
%7 = inttoptr i32 %6 to ptr ; [#uses = 2]
store volatile ptr %7, ptr %stackSpill.unnamedVreg, align 4
tail call void @_D9test_libc6usePtrFPvZv(ptr %7) #1
ret void
}Even though with Though in this example, I guess dumping Maybe it's a narrower surface than I thought. Since expression chains are trees. Maybe just dumping (rets and args) and |
|
Even if I can correctly track down everywhere in the front-end that needs spilling and anchoring, it feels that #5178 is better contained, less likely to miss things we maybe didn't think off (if we're really paranoid, we could just mark ALL vreg as potential pointers; act more like native platforms with stack & register spilling), and interferes with the optimizer less (pinning alloca optimizations won't happen on aggregates, even if only a small part of a large struct is used as a pointer). |
I didn't mean to criticise any of your PRs, but instead wanted to give you some support that it is OK if the implementation has "poor" or pessimistic performance, whichever you decide on in the end. :) |
|
Closing in favor of #5178 (could always be reopened...) |
3 participants
On WebAssembly, a working conservative GC is currently blocked on being able to see pointers that might be held somewhere outside of Wasm's linear memory.
Wasm has both an infinite set of "locals" (~registers), AND it's own value stack (Wasm is a stack machine). Neither of these are easily introspectable from arbitrary code. Unlike native platforms, we can't just spill all the registers onto the stack and be done with it.
Furthermore, due largely to
union, there may be pointers smuggled in types other than the appropriatei32onwasm32ori64onwasm64. E.g.union U { double d; void* p }holds a pointer in what LLVM sees as just a double (and becomes Wasmf64).To work around these limitations, this PR adds some more code to
allocageneration (DtoAlloca*)Whenever an
allocais created for a D type thathasPointers, a call to an inline assemblynopis inserted, passing theallocaalong as a parameter with register/rconstraint. This inhibitsSROAandmem2reg, forcing the value to remain on the stack. Rather than spilling things back, we simply force them to stay. Otherwise thenopis cheap if not free, and tools likewasm-optcan safely clean it up after.LLVM assumes
captureson the pointer by default, which makes sure that anystores are visible beforecalls (which LLVM will assume might read the pointer). When available (>= LLVM 21),captures(read_provenance)is specified to avoid forcing reloadafter such calls (it is assumed the D does not use a moving GC, so we can avoid this pessimization; e.g. store forwarding optimizations still work across calls).These calls are not inserted on
@nogcfunctions (and in nogc compilations, e.g.betterC). This relies on the transitivity of@nogc. If a function is@nogc, it may not call non-@nogcfunctions, which we assume to meanGC.collectwill never be invoked.