[PATCH] D73597: [LTO] Add optimization remarks for removed functions

[Gábor Horváth via Phabricator via llvm-commits]

xazax.hun marked 2 inline comments as done.
xazax.hun added inline comments.


================
Comment at: llvm/lib/LTO/LTO.cpp:967
+
+  // Ensure we don't have inconsistently split LTO units with type tests.
+  if (Error Err = checkPartiallySplit())
----------------
tejohnson wrote:
> Can you add a FIXME here noting that this is checking both Regular and Thin LTO and should be split into two, one checking Regular here and the other checking the ThinLTO index in runThinLTO?
> 
> This does work since we always go through this path, but logically it would make more sense to do the ThinLTO checking in runThinLTO. Unfortunately, the regular LTO checking this is doing must be after linkRegularLTO.
Let me know if you prefer to reword this. I am not a native speaker and not familiar with the nomenclature of LLVM.  


================
Comment at: llvm/test/LTO/Resolution/X86/dead-strip-fulllto.ll:19
 
+; REMARK: - Function:        dead2
+
----------------
tejohnson wrote:
> Is the yaml remark more verbose than this (i.e. it includes your message)? Would be good to include more of it.
Good point, thanks!


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

https://reviews.llvm.org/D73597