rnk added a comment.

+@aprantl @dblaikie

The basic idea of this change is that clang should store implicit sret pointer 
arguments into a static alloca, and the dbg.declare should reference the static 
alloca instead of referring to the LLVM argument directly. The idea is that 
that will be more resilient. The current consensus seems to be that that's the 
best way to ensure we have maximally correct debug info at -O0. Let us know if 
you have any feedback on how to do this.



================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3940-3941
 
+  // Add DW_OP_deref to NRVO variables because we set their debug values to be
+  // references.
+  if (VD->isNRVOVariable())
----------------
I wouldn't mention references here since those are codeview specific. This 
should instead say something like: "Clang stores the sret pointer provided by 
the caller in a static alloca. Use DW_OP_deref to tell the debugger to load 
that pointer and treat it as the address of the variable.".


================
Comment at: clang/lib/CodeGen/CGDecl.cpp:1566-1576
+    // NRVO variables are stored in the return register and are not always
+    // readable by the debugger. Get around this by storing a reference to
+    // the variable.
+    if (NRVO) {
+      llvm::Type *RefTy =
+          ConvertTypeForMem(getContext().getLValueReferenceType(Ty));
+      DebugAddr = CreateTempAlloca(RefTy, getPointerAlign(), "nrvo_ref");
----------------
I think this alloca is something that we should set up in the prologue. Take a 
look at CodeGenFunction::StartFunction for where the ReturnValue member 
variable is initialized. You can create the alloca there, save it, and reuse it 
here for every NRVO variable, since there could be more than one, and we they 
can all use the same static alloca.

Initially I thought perhaps we could change the meaning of `ReturnValue` to 
simply *be* this alloca, but I don't think it will be so easy. I'd recommend 
adding a second member next to it that's null unless sret is in use.

Finally, I notice we need to do something if inalloca is involved. (Yuck.) You 
can see how its handled in StartFunction. You could use the result of the 
Builder.CreateStructGEP call here as the value to save to pass to dbg.declare 
when NRVO fires:
https://github.com/llvm/llvm-project/blob/14059d2a13667409ccf2860ff7d1c56f6c8c70a6/clang/lib/CodeGen/CodeGenFunction.cpp#L904


================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:1145-1149
+    bool Deref = false;
+    if (VI.Expr) {
+      if (!VI.Expr->extractIfOffset(ExprOffset, Deref))
         continue;
+    }
----------------
I see. I thought this code was using the stuff I added in 
`DbgVariableLocation::extractFromMachineInstruction`. Hm.

In practice, do you actually see DIExpression offsets here? I think maybe we 
should just special case a single DW_OP_deref expression, since that's what we 
get at O0.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63361/new/

https://reviews.llvm.org/D63361



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to