[PATCH] D107034: [CSPGO] Set EnableValueProfiling flag for CSPGO to avoid IRPGO value loss in LTO

Fri Aug 20 00:37:29 PDT 2021

YolandaCY added a comment.

In D107034#2956730 <https://reviews.llvm.org/D107034#2956730>, @MaskRay wrote:

> About this patch `M.addModuleFlag(llvm::Module::Warning, "EnableValueProfiling", 1);`: I am not so sure we should do it.

>From `enablesValueProfiling` both IRPGOFlag and EnableValueProfiling mudule flag are examined as indicators. My understanding IRPGOFlag should imply "EnableValueProfiling" to 1.

This alternative solution was recommended by Reid, and no objection from Xu. Could you help clarify with Fangrui? @rnk @xur

> Note that we only set "EnableValueProfiling" to 0 for the `clang -fprofile-instr-generate` case. The value is not set to 1 (without poking into the internal `-enable-value-profiling` option).
>
> `clang -fprofile-generate` with value profiling doesn't set this module flag, so -fcs-profile-generate doesn't need to set it, either.

I think I have clarified why this change is unnecessary to PGO in previous reply.

I can drop the unrelated change regarding /lldsavetemps for sure. That should be an easy fix.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107034