[PATCH] D109737: [LTO] Emit DebugLoc for dead function in optimization remarks

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: llvm/lib/LTO/LTO.cpp:860
       if (Function *F = dyn_cast<Function>(GV)) {
+        if (Error Err = F->materialize())
+          return Err;
----------------
Enna1 wrote:
> tejohnson wrote:
> > Were we eventually materializing these dead functions without this change? If not, this seems like a compile time degradation. Would it work to move the materialize call later, capture the OptimizationRemark in a variable, and guard the materialization by isEnabled() on the remark?
> Thanks for your comment!
> Yes, this will cause a compile time degradation, without this change these dead function will never be materialized.
> To emit DebugLoc for dead function in optimization remarks, we must call `F->materialize` before `ORE(F, nullptr);` and `ore::NV("Function", F)`.
> I'm looking for a variable which indicates whether we enable emit OptimizationRemark or not, we can use such variable to guard the materialization.
> Here are some solutions came up to me:
> 1. Check if `Conf.RemarksFilename` is empty, only materializing these dead functions when `Conf.RemarksFilename` is not empty. 
> 2. In `LTO::linkRegularLTO` collect all dead functions, and in `lto::finalizeOptimizationRemarks` call materialize and OptimizationRemarkEmitter for all dead functions. This will introduce a variable to store all dead functions.
> Thanks again!
Ah, it also looks like the Function will need to be materialized before the OptimizationRemark constructor call, which was what my suggested solution would have required. Your solution looks ok to me.


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

https://reviews.llvm.org/D109737