[PATCH] D136180: [FuncSpec] Compute specialisation gain even when forcing specialisation

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added a comment.

The patch itself look good to me. And it looks like the fundamental problem is that we'll match different specializations with a single callsties. Although we workaround it, it'll be better solve the root problems.



================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:456
     // Remove unprofitable specializations.
-    Specializations.remove_if(
-        [](const auto &Entry) { return Entry.second.Gain <= 0; });
+    if (!ForceFunctionSpecialization)
+      Specializations.remove_if(
----------------
labrinea wrote:
> I can see that you are not removing unprofitable specializations, which means you've preserved the "force" semantics of the command line option. Have you checked whether any of the existing tests breaks with this change?
It looks like we can preserve the current semantics since we don't check the Gains further. It is a little bit odd if we'll specialize the callosities of which has Gain less than 0. But the author explains "assumption that this flag has purely debugging
purpose and it is reasonable to ignore the extra computing effort it incurs." And this explanation looks reasonable to me.


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

https://reviews.llvm.org/D136180