[llvm-commits] [llvm] r105528 - in /llvm/trunk: lib/Transforms/IPO/PartialSpecialization.cpp test/Transforms/PartialSpecialize/ test/Transforms/PartialSpecialize/dg.exp test/Transforms/PartialSpecialize/two-specializations.ll

Daniel Dunbar daniel at zuster.org
Wed Jun 23 11:03:11 PDT 2010


Hi Kenneth,

This test is failing on llvm-arm-linux. It looks to me like the order
of the partial specialization numbering isn't deterministic?

Failure is here:
  http://google1.osuosl.org:8011/builders/llvm-arm-linux/builds/3516/steps/test-llvm/logs/two-specializations.ll

Can you take a look?

 - Daniel

On Sat, Jun 5, 2010 at 7:50 AM, Kenneth Uildriks <kennethuil at gmail.com> wrote:
> Author: kennethuil
> Date: Sat Jun  5 09:50:21 2010
> New Revision: 105528
>
> URL: http://llvm.org/viewvc/llvm-project?rev=105528&view=rev
> Log:
> Partial specialization was not checking the callsite to make sure it was using the same constants as the specialization, leading to calls to the wrong specialization.  Patch by Takumi Nakamura\!
>
> Added:
>    llvm/trunk/test/Transforms/PartialSpecialize/
>    llvm/trunk/test/Transforms/PartialSpecialize/dg.exp
>    llvm/trunk/test/Transforms/PartialSpecialize/two-specializations.ll
> Modified:
>    llvm/trunk/lib/Transforms/IPO/PartialSpecialization.cpp
>
> Modified: llvm/trunk/lib/Transforms/IPO/PartialSpecialization.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/PartialSpecialization.cpp?rev=105528&r1=105527&r2=105528&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/IPO/PartialSpecialization.cpp (original)
> +++ llvm/trunk/lib/Transforms/IPO/PartialSpecialization.cpp Sat Jun  5 09:50:21 2010
> @@ -32,6 +32,10 @@
>  using namespace llvm;
>
>  STATISTIC(numSpecialized, "Number of specialized functions created");
> +STATISTIC(numReplaced, "Number of callers replaced by specialization");
> +
> +// Maximum number of arguments markable interested
> +static const int MaxInterests = 6;
>
>  // Call must be used at least occasionally
>  static const int CallsMin = 5;
> @@ -40,8 +44,9 @@
>  static const double ConstValPercent = .1;
>
>  namespace {
> +  typedef SmallVector<int, MaxInterests> InterestingArgVector;
>   class PartSpec : public ModulePass {
> -    void scanForInterest(Function&, SmallVector<int, 6>&);
> +    void scanForInterest(Function&, InterestingArgVector&);
>     int scanDistribution(Function&, int, std::map<Constant*, int>&);
>   public :
>     static char ID; // Pass identification, replacement for typeid
> @@ -61,11 +66,13 @@
>  SpecializeFunction(Function* F,
>                    DenseMap<const Value*, Value*>& replacements) {
>   // arg numbers of deleted arguments
> -  DenseSet<unsigned> deleted;
> +  DenseMap<unsigned, const Argument*> deleted;
>   for (DenseMap<const Value*, Value*>::iterator
>          repb = replacements.begin(), repe = replacements.end();
> -       repb != repe; ++repb)
> -    deleted.insert(cast<Argument>(repb->first)->getArgNo());
> +       repb != repe; ++repb) {
> +    Argument const *arg = cast<const Argument>(repb->first);
> +    deleted[arg->getArgNo()] = arg;
> +  }
>
>   Function* NF = CloneFunction(F, replacements);
>   NF->setLinkage(GlobalValue::InternalLinkage);
> @@ -80,9 +87,23 @@
>       if (CS.getCalledFunction() == F) {
>
>         SmallVector<Value*, 6> args;
> -        for (unsigned x = 0; x < CS.arg_size(); ++x)
> -          if (!deleted.count(x))
> -            args.push_back(CS.getArgument(x));
> +        // Assemble the non-specialized arguments for the updated callsite.
> +        // In the process, make sure that the specialized arguments are
> +        // constant and match the specialization.  If that's not the case,
> +        // this callsite needs to call the original or some other
> +        // specialization; don't change it here.
> +        CallSite::arg_iterator as = CS.arg_begin(), ae = CS.arg_end();
> +        for (CallSite::arg_iterator ai = as; ai != ae; ++ai) {
> +          DenseMap<unsigned, const Argument*>::iterator delit = deleted.find(
> +            std::distance(as, ai));
> +          if (delit == deleted.end())
> +            args.push_back(cast<Value>(ai));
> +          else {
> +            Constant *ci = dyn_cast<Constant>(ai);
> +            if (!(ci && ci == replacements[delit->second]))
> +              goto next_use;
> +          }
> +        }
>         Value* NCall;
>         if (CallInst *CI = dyn_cast<CallInst>(i)) {
>           NCall = CallInst::Create(NF, args.begin(), args.end(),
> @@ -99,8 +120,11 @@
>         }
>         CS.getInstruction()->replaceAllUsesWith(NCall);
>         CS.getInstruction()->eraseFromParent();
> +        ++numReplaced;
>       }
>     }
> +  next_use:
> +    ;
>   }
>   return NF;
>  }
> @@ -111,7 +135,7 @@
>   for (Module::iterator I = M.begin(); I != M.end(); ++I) {
>     Function &F = *I;
>     if (F.isDeclaration() || F.mayBeOverridden()) continue;
> -    SmallVector<int, 6> interestingArgs;
> +    InterestingArgVector interestingArgs;
>     scanForInterest(F, interestingArgs);
>
>     // Find the first interesting Argument that we can specialize on
> @@ -143,7 +167,7 @@
>
>  /// scanForInterest - This function decides which arguments would be worth
>  /// specializing on.
> -void PartSpec::scanForInterest(Function& F, SmallVector<int, 6>& args) {
> +void PartSpec::scanForInterest(Function& F, InterestingArgVector& args) {
>   for(Function::arg_iterator ii = F.arg_begin(), ee = F.arg_end();
>       ii != ee; ++ii) {
>     for(Value::use_iterator ui = ii->use_begin(), ue = ii->use_end();
>
> Added: llvm/trunk/test/Transforms/PartialSpecialize/dg.exp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/PartialSpecialize/dg.exp?rev=105528&view=auto
> ==============================================================================
> --- llvm/trunk/test/Transforms/PartialSpecialize/dg.exp (added)
> +++ llvm/trunk/test/Transforms/PartialSpecialize/dg.exp Sat Jun  5 09:50:21 2010
> @@ -0,0 +1,3 @@
> +load_lib llvm.exp
> +
> +RunLLVMTests [lsort [glob -nocomplain $srcdir/$subdir/*.{ll,c,cpp}]]
>
> Added: llvm/trunk/test/Transforms/PartialSpecialize/two-specializations.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/PartialSpecialize/two-specializations.ll?rev=105528&view=auto
> ==============================================================================
> --- llvm/trunk/test/Transforms/PartialSpecialize/two-specializations.ll (added)
> +++ llvm/trunk/test/Transforms/PartialSpecialize/two-specializations.ll Sat Jun  5 09:50:21 2010
> @@ -0,0 +1,32 @@
> +; If there are two specializations of a function, make sure each callsite
> +; calls the right one.
> +;
> +; RUN: opt -S -partialspecialization %s | FileCheck %s
> +declare void @callback1()
> +declare void @callback2()
> +
> +define internal void @UseCallback(void()* %pCallback) {
> +  call void %pCallback()
> +  ret void
> +}
> +
> +define void @foo(void()* %pNonConstCallback)
> +{
> +Entry:
> +; CHECK: Entry
> +; CHECK-NEXT: call void @UseCallback1()
> +; CHECK-NEXT: call void @UseCallback1()
> +; CHECK-NEXT: call void @UseCallback2()
> +; CHECK-NEXT: call void @UseCallback(void ()* %pNonConstCallback)
> +; CHECK-NEXT: call void @UseCallback1()
> +; CHECK-NEXT: call void @UseCallback2()
> +; CHECK-NEXT: call void @UseCallback2()
> +  call void @UseCallback(void()* @callback1)
> +  call void @UseCallback(void()* @callback1)
> +  call void @UseCallback(void()* @callback2)
> +  call void @UseCallback(void()* %pNonConstCallback)
> +  call void @UseCallback(void()* @callback1)
> +  call void @UseCallback(void()* @callback2)
> +  call void @UseCallback(void()* @callback2)
> +  ret void
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>




More information about the llvm-commits mailing list