[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

This revision was automatically updated to reflect the committed changes. Closed by commit rGa2a3ca8d9796: BPF: emit debuginfo for Function of DeclRefExpr if requested (authored by yonghong-song). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100567/

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:2844 + auto *Fn = + dyn_cast(LV.getPointer(*this)->stripPointerCasts()); + if (DI && !Fn->getSubprogram()) dblaikie wrote: > Oh, please change this to cast, rather th

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song updated this revision to Diff 340675. yonghong-song added a comment. formatting to mode "DI = CGM.getModuleDebugInfo()" and use cast<...> instead of dyn_cast<...> since there should be no possibility of null ptr. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

dblaikie added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:2844 + auto *Fn = + dyn_cast(LV.getPointer(*this)->stripPointerCasts()); + if (DI && !Fn->getSubprogram()) Oh, please change this to cast, rather than dyn_cast before comm

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

dblaikie accepted this revision. dblaikie added a comment. Looks good - thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100567/new/ https://reviews.llvm.org/D100567 ___ cfe-commits mailing list cfe

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song updated this revision to Diff 340650. yonghong-song added a comment. - use stripPointerCasts() to remove the cast of DeclRefExpr pointer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100567/new/ https://reviews.llvm.org/D100567 Files

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song added a comment. Looks like stripPointerCasts() is enough to make it work (no null Function pointer any more). This is consistent with llvm::Constant *CodeGenModule::GetOrCreateLLVMFunction( StringRef MangledName, llvm::Type *Ty, GlobalDecl GD, bool ForVTable, bool Do

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

dblaikie added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:2843-2844 + CGDebugInfo *DI = CGM.getModuleDebugInfo(); + auto *Fn = dyn_cast(LV.getPointer(*this)); + if (DI && Fn && !Fn->getSubprogram()) +DI->EmitFunctionDecl(FD, FD->getLocation()

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:2843-2844 + CGDebugInfo *DI = CGM.getModuleDebugInfo(); + auto *Fn = dyn_cast(LV.getPointer(*this)); + if (DI && Fn && !Fn->getSubprogram()) +DI->EmitFunctionDecl(FD, FD->getLocat

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song updated this revision to Diff 340606. yonghong-song added a comment. - somehow previous test case didn't trigger nullptr for Fn. Replaced it with a working test case. With the following change, diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index a784

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

dblaikie added a comment. (ah, sorry, failed to submit comment) Comment at: clang/lib/CodeGen/CGExpr.cpp:2843-2844 + CGDebugInfo *DI = CGM.getModuleDebugInfo(); + auto *Fn = dyn_cast(LV.getPointer(*this)); + if (DI && Fn && !Fn->getSubprogram()) +DI->Emit

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song added a comment. @dblaikie ping. Did you get a chance of looking at it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100567/new/ https://reviews.llvm.org/D100567 ___ cfe-commits mailing li

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

dblaikie added a comment. In D100567#2702394 , @yonghong-song wrote: > ping @dblaikie could you take a look of my new investigation? Yep, it's on my list to look at. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song added a comment. ping @dblaikie could you take a look of my new investigation? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100567/new/ https://reviews.llvm.org/D100567 ___ cfe-commits mai

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song added a comment. @dblaikie I did some investigation, found the root cause and provided an alternative solution. Could you check whether you are okay with either approach? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100567/new/ http

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song added a comment. Did some further investigation, the following is the callchain: EmitFunctionDeclLValue EmitFunctionDeclPointer GetAddrOfFunction GetOrCreateLLVMFunction The GetOrCreateLLVMFunction has the following comments: /// GetOrCreateLLVMFunct

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

dblaikie added a comment. In D100567#2697415 , @yonghong-song wrote: > To answer one of your questions above, if there is a function definition > before, dyn_cast(...) will return nullptr. I tried starting > from "Value" class and found dyn_cast to llv

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song added a comment. To answer one of your questions above, if there is a function definition before, dyn_cast(...) will return nullptr. I tried starting from "Value" class and found dyn_cast to llvm::ConstantExpr will result in a non-null object, but I did not trace down further with

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song added a comment. In D100567#2697412 , @dblaikie wrote: > In D100567#2696095 , @yonghong-song > wrote: > >> Sorry, I know what is the segfault now after some debugging. It is because >> `auto *Fn =

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

dblaikie added a comment. In D100567#2696095 , @yonghong-song wrote: > Sorry, I know what is the segfault now after some debugging. It is because > `auto *Fn = dyn_cast(LV.getPointer(*this));` is a NULL > pointer after there is a definition before this

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song added a comment. I did some debugging on why DeclRefExpr evaluated not to be a Function pointer. Here is what I got. I made the following clang change to dump out the pointer to the emited value ('&foo'): diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp ind

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song updated this revision to Diff 338323. yonghong-song added a comment. - add checking CGDebugInfo pointer. In my original patch but got lost in the revision. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100567/new/ https://reviews.llv

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song updated this revision to Diff 338252. yonghong-song added a comment. - isDefined() is not necessary, segfault is caused by a nullptr by causing a LV.getPointer(). Check nullptr instead. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D10

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song added a comment. Sorry, I know what is the segfault now after some debugging. It is because `auto *Fn = dyn_cast(LV.getPointer(*this));` is a NULL pointer after there is a definition before this. Will pose another patch but with test case remains to cover mix of declaration, point

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song added a comment. In D100567#2695827 , @dblaikie wrote: > Generally looks OK, but could you explain more about the problems > with/motivation for the declaration/definition issue? (probably worth noting > in the patch description maybe comm

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

dblaikie added a comment. Generally looks OK, but could you explain more about the problems with/motivation for the declaration/definition issue? (probably worth noting in the patch description maybe comment in code and test) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song added a comment. @dblaikie I renamed the variable, added a few guards to ensure safety and avoid unnecessary attempt to generate duplicated debuginfo, and enhanced the test case to cover mix of declaration, accessing extern func address and definition of the function. Could you ta

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song updated this revision to Diff 338202. yonghong-song added a comment. fix a clang-format warning Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100567/new/ https://reviews.llvm.org/D100567 Files: clang/include/clang/Basic/TargetInfo.

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song updated this revision to Diff 338002. yonghong-song added a comment. - checked both !FD->isDefined() and !Fn->getSubprogram() before emitting debuginfo for the declaration - added additional tests to mix DeclRefExpr and actual function definition. Repository: rG LLVM Github Mono

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song updated this revision to Diff 337993. yonghong-song added a comment. - check Fn->getSubprogram() before emit debuginfo. This is consistent with EmitFuncDeclForCallSite() and is better than checking FD->isDefined() as we may hit multiple DeclRefExpr and all of them will go through e

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song updated this revision to Diff 337988. yonghong-song added a comment. check FD->isDefined() as well before emit debuginfo for the declaration. It is okay to emit a declaration subprogram and later refined to be with definition. But it is not okay to refine a definition to a declarat

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song updated this revision to Diff 337948. yonghong-song edited the summary of this revision. yonghong-song added a comment. Rename TargetInfo.allowDebugInfoForExternalVar to TargetInfo.allowDebugInfoForExternalRef. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

dblaikie added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:2840 +// Emit debuginfo for the function declaration if the target wants to. +if (getContext().getTargetInfo().allowDebugInfoForExternalVar()) { + CGDebugInfo *DI = CGM.getModuleDebugInfo(); ---

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:2840 +// Emit debuginfo for the function declaration if the target wants to. +if (getContext().getTargetInfo().allowDebugInfoForExternalVar()) { + CGDebugInfo *DI = CGM.getModuleDebugInfo()

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

dblaikie added a comment. ah, right, because this is powered by seeing the DeclRefExpr only in code that's codegen'd - fair enough. Thanks for checking! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100567/new/ https://reviews.llvm.org/D100567 __

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song added a comment. For the first example, actually clang is smart enough to remove all dead code, so nothing generated. [yhs@devbig003.ftw2 ~/tmp/ext_func_var]$ cat t1.c extern void f1(); void f2(void *); inline void f3() { f2(f1); } [yhs@devbig003.ftw2 ~/tmp/ext_func_var]$ clang

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

dblaikie added a comment. What happens for this program: extern void f1(); void f2(void *); inline void f3() { f2(f1); } ... Even when `f3` is never called, I'm guessing your change will cause `f1` to be emitted? Also something like this: void f1(); int main() { int x =

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song added a comment. Yes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100567/new/ https://reviews.llvm.org/D100567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

anakryiko accepted this revision. anakryiko added a comment. Nice, thanks! This will work for externs with explicit section name (.ksym) and with no section name (externs for static linking), right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song added a comment. The corresponding clang patch is here: https://reviews.llvm.org/D100567 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100567/new/ https://reviews.llvm.org/D100567 ___ cfe-c

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

ast accepted this revision. ast added a comment. This revision is now accepted and ready to land. Makes sense to me. Only BPF target will notice this difference. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100567/new/ https://reviews.llvm.org/D10

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

yonghong-song created this revision. yonghong-song added a reviewer: dblaikie. yonghong-song added projects: debug-info, clang. yonghong-song requested review of this revision. Herald added a subscriber: cfe-commits. Commit e3d8ee35e4ad