[PATCH] D104461: [FuncSpec] Don't specialize functions with NoDuplicate instructions.

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: ChuanqiXu, fhahn, jaykang10.
Herald added subscribers: ormris, hiraditya.
SjoerdMeijer requested review of this revision.
Herald added a project: LLVM.

getSpecializationCost was returning INT_MAX for a case when specialisation shouldn't happen, but this wasn't properly checked with specialisation was forced.


https://reviews.llvm.org/D104461

Files:
  llvm/lib/Transforms/IPO/FunctionSpecialization.cpp
  llvm/test/Transforms/FunctionSpecialization/function-specialization-nodup2.ll


Index: llvm/test/Transforms/FunctionSpecialization/function-specialization-nodup2.ll
===================================================================
--- llvm/test/Transforms/FunctionSpecialization/function-specialization-nodup2.ll
+++ llvm/test/Transforms/FunctionSpecialization/function-specialization-nodup2.ll
@@ -1,12 +1,12 @@
 ; RUN: opt -function-specialization -force-function-specialization -S < %s | FileCheck %s
 
-; FIXME: Function foo gets specialised even though it contains an instrinsic
+; Check that function foo does not gets specialised as it contains an instrinsic
 ; that is marked as NoDuplicate.
 ; Please note that the use of the hardwareloop intrinsic is arbitrary; it's
 ; just an easy to use intrinsic that has NoDuplicate.
 
-; CHECK: @foo.1(
-; CHECK: @foo.2(
+; CHECK-NOT: @foo.1(
+; CHECK-NOT: @foo.2(
 
 target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
 
Index: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp
===================================================================
--- llvm/lib/Transforms/IPO/FunctionSpecialization.cpp
+++ llvm/lib/Transforms/IPO/FunctionSpecialization.cpp
@@ -248,9 +248,12 @@
       Metrics.analyzeBasicBlock(&BB, (GetTTI)(*F), EphValues);
 
     // If the code metrics reveal that we shouldn't duplicate the function, we
-    // shouldn't specialize it. Set the specialization cost to the maximum.
-    if (Metrics.notDuplicatable)
-      return std::numeric_limits<unsigned>::max();
+    // shouldn't specialize it. Set the specialization cost to Invalid.
+    if (Metrics.notDuplicatable) {
+      InstructionCost C{};
+      C.setInvalid();
+      return C;
+    }
 
     // Otherwise, set the specialization cost to be the cost of all the
     // instructions in the function and penalty for specializing more functions.
@@ -417,6 +420,11 @@
     // function where the argument takes on the given constant value. If so,
     // add the constant to Constants.
     auto FnSpecCost = getSpecializationCost(F);
+    if (!FnSpecCost.isValid()) {
+      LLVM_DEBUG(dbgs() << "FnSpecialization: Invalid specialisation cost.\n");
+      return false;
+    }
+
     LLVM_DEBUG(dbgs() << "FnSpecialization: func specialisation cost: ";
                FnSpecCost.print(dbgs()); dbgs() << "\n");
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D104461.352708.patch
Type: text/x-patch
Size: 2289 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210617/4a9cd8cf/attachment.bin>