[llvm] [FuncSpec] Enable SpecializeLiteralConstant by default (PR #113442)

Alexandros Lamprineas via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 24 04:32:30 PDT 2024


================
@@ -682,10 +679,9 @@ bool FunctionSpecializer::run() {
         (RequireMinSize && Metrics.NumInsts < MinFunctionSize))
       continue;
 
-    // TODO: For now only consider recursive functions when running multiple
-    // times. This should change if specialization on literal constants gets
-    // enabled.
-    if (!Inserted && !Metrics.isRecursive && !SpecializeLiteralConstant)
+    // When specialization on literal constants is disabled, only consider
+    // recursive functions when running multiple times.
+    if (!SpecializeLiteralConstant && !Inserted && !Metrics.isRecursive)
----------------
labrinea wrote:

You changed the comment but not the condition (other than reordering it). Perhaps the TODO wasn't clear when I wrote it. Sorry. I meant to say that when we allow specializations on literal constants, a specialization may return a constant. Therefore the SCCP Solver needs to invalidate the LatticeValue for the users of that returned value https://github.com/llvm/llvm-project/blob/c07abf727220e3bd2c78b129a683ad79f735e43c/llvm/lib/Transforms/IPO/FunctionSpecialization.cpp#L804 and to recalculate it https://github.com/llvm/llvm-project/blob/c07abf727220e3bd2c78b129a683ad79f735e43c/llvm/lib/Transforms/IPO/FunctionSpecialization.cpp#L810

So we used to only consider re-evaluating recursive functions on subsequent iterations of the Specializer, but now we should consider everything. I think when I wrote the TODO I was thinking that we may want to remove this check completely, but it make sense to keep it in case SpecializeLiteralConstant=false to avoid repeating the same computations since no lattice value could have changed from the previous iteration. Can you reflect what I wrote here in the comment? Reordering the conditions seem fine to me.

https://github.com/llvm/llvm-project/pull/113442


More information about the llvm-commits mailing list