rnk added a comment.

I see we don't have any tests for inalloca to model this on, so I think we 
should skip that for this change. I'll add one later that handles arguments as 
well, since those are interesting.



================
Comment at: clang/lib/CodeGen/CGDecl.cpp:1564
+    Address DebugAddr = address;
+    bool UsePointerValue = false;
     DI->setLocation(D.getLocation());
----------------
Nit: Since the boolean is really the condition of the if, I'd suggest writing 
it like this:
  bool UsePointerValue = NRVO && ReturnValuePointer.isValid();
  if (UsePointerValue)
    DebugAddr = ReturnValuePointer;
  ...
It's more functional, less stateful, and is one less line in the end.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:908
     --EI;
     llvm::Value *Addr = Builder.CreateStructGEP(nullptr, &*EI, Idx);
     Addr = Builder.CreateAlignedLoad(Addr, getPointerAlign(), "agg.result");
----------------
akhuang wrote:
> rnk wrote:
> > What I had in mind was to use this GEP as the ReturnValuePointer here. The 
> > inalloca parameter is also a pointer to stack memory, and a GEP is an 
> > offset, so it should end up being handled like a static alloca.
> Oh, ok. I changed it, but not sure how to test debug info for the inalloca 
> case
I don't think this is the correct alignment. The sret slot in the inalloca has 
a pointer type, so I think you want pointer alignment here. I think you can 
substitute Int8PtrTy for RetTy here.


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