[PATCH] D108581: [CSPGO] Fix lost IRPGOFlag in CSPGO instrumentation

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

> This is still problematic because we currently query this symbol to coordinate some actions B/W PGOInstrumentation and InstroProfiling lowering, like whether to do value profiling, weather to do comdat renaming.

Worth clarifying B/W.

---

I have confirmed that

In regular LTO, the non-prevailing variable is dropped unless its linkage weak_odr/linkonce_odr && `-compute-dead=0`
In ThinLTO, the variable is changed to available_external linkage by `thinLTOResolvePrevailingGUID` then dropped by GlobleOpt:deleteIfDead in the backend.

Does it make sense to update `thinlto_cspgo_gen.ll` as well?



================
Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:469
+    appendToCompilerUsed(
+        M, createIRLevelProfileFlagVar(M, /* IsCS */ true, PGOInstrumentEntry));
     return false;
----------------
The form liked by both clang-tidy bugprone-argument-comment and clang-format: `/*IsCS=*/true`


================
Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1638
+  appendToCompilerUsed(
+      M, createIRLevelProfileFlagVar(M, /* IsCS */ true, PGOInstrumentEntry));
   return PreservedAnalyses::all();
----------------
The form liked by both clang-tidy bugprone-argument-comment and clang-format: `/*IsCS=*/true`


================
Comment at: llvm/test/Transforms/PGOProfile/lto_cspgo_gen.ll:8
+; RUN:   -r=%t.bc,f,px \
+; RUN:   -r=%t.bc,__llvm_profile_filename,x \
+; RUN:   -r=%t.bc,__llvm_profile_raw_version,x
----------------
Worth a comment that the two variables are intentionally non-prevailing.


================
Comment at: llvm/test/Transforms/PGOProfile/lto_cspgo_gen.ll:26
+
+attributes #0 = { "target-cpu"="x86-64" }
+
----------------
unneeded and can be dropped


================
Comment at: llvm/test/Transforms/PGOProfile/lto_cspgo_gen.ll:31
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{i32 7, !"uwtable", i32 1}
+!2 = !{i32 1, !"ThinLTO", i32 0}
----------------
unneeded and can be dropped


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

https://reviews.llvm.org/D108581