[PATCH] D146543: [Coroutines] Look for dbg.declare for temp spills
Felipe de Azevedo Piovezan via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 25 16:42:36 PDT 2023
fdeazeve added a comment.
Thank you for the quick response!
>> Note that this will crash the compiler if CurDef is a GlobalVariable.
>
> You are right, and we also observed the crash and fixed in https://reviews.llvm.org/D157423. Since the original change is targeting a very specific case, the fix puts more restriction on the case it can affect.
Thank you for the pointers, glad to hear it was fixed in a subsequent commit. I was working on a slightly older version of the codebase.
> Do you have any suggestions on how can we make it more generic and applicable to more use cases?
> After:
>
> define internal fastcc void @foo.resume(ptr noundef nonnull align 8 dereferenceable(32) %hdl) !dbg !38 {
> entry.resume:
> %hdl.debug = alloca ptr, align 8
> call void @llvm.dbg.declare(metadata ptr %hdl.debug, metadata !41, metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst, 24)), !dbg !42
> call void @llvm.dbg.declare(metadata ptr %hdl.debug, metadata !40, metadata !DIExpression(DW_OP_deref)), !dbg !42
>
> ... ...
>
> !40 = !DILocalVariable(name: "__coro_frame", scope: !38, file: !1, type: !22, flags: DIFlagArtificial)
> !41 = !DILocalVariable(name: "this", arg: 1, scope: !38, type: !35, flags: DIFlagArtificial | DIFlagObjectPointer)
I understand the problem the patch is solving, but I haven't been able to figure out how we are going from that dbg.declare that I mentioned to the one that ends up in the final output of the compiler. Do you see the problem that I was trying to point to? The _first_ dbg.declare that we create -- the one shown in the output of `p CurrentBlock->dump()` is not equivalent to the original dbg.declare. And yet, somehow, this "invalid" dbg.declare disappears and the correct one shows up later. To be very specific, when we do:
DIBuilder(*CurrentBlock->getParent()->getParent(), AllowUnresolved)
.insertDeclare(CurrentReload, DDI->getVariable(),
DDI->getExpression(), DDI->getDebugLoc(),
&*Builder.GetInsertPoint());
The `CurrentReload` seems incorrect if we stripped some of the loads along the way. Does that make sense?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146543/new/
https://reviews.llvm.org/D146543
More information about the llvm-commits
mailing list