getScalarizationOverhead(), revisited

[Hal Finkel via llvm-commits]

On 01/28/2017 02:21 AM, Jonas Paulsson wrote:
> Hi Hal,
>
>> Yes. LGTM.
>>
>
> Sorry, I should have run all the regression tests before asking for 
> opinion, and I see now that this actually fails a test:
> test/Transforms/LoopVectorize/X86/funclet.ll
>
> Cost += getScalarizationOverhead(VectorType::get(A->getType(), VF), 
> false, true);
> asserts, because A is a token.
>
> , the call
> %call = call double @floor(double 1.000000e+00) #1 [ "funclet"(token 
> %1) ]
>
> has the function signature and token as operands also, so I added a 
> check so that for a CallInst only the call arguments are passed.
>
> Then I also realized that we should not consider costs for extracting 
> constant values, so I added a check for this also in 
> getOperandsScalarizationOverhead(). 

Sounds good (that's not in the diff but certainly makes sense).

> However, as a result now the same test fails again, because since the 
> getVectorIntrinsicCost() method (as discussed above) has not been fixed
> yet, the vectorizer now thinks it is not beneficial anymore to 
> vectorize the intrinsic call.
>
> I started looking at this then, and a first question is why the two 
> versions of getIntrinsicInstrCost() do not have equivalent behavior? 
> This is confusing to me, as I was hoping to just remove the version 
> with the argument *types* passed, and just use the one with the actual 
> arguments passed. I would have expected that the types handling 
> version would be a subset of the function getting actual arguments.

I agree.

> I see a fixme comment for cttz in the arguments version, which are 
> handled in the types version, so perhaps it would be a first step to 
> make sure this is the case?

Yes, we should clean this up.

> Or would it be right to merge these two into one that accepts the 
> actual arguments as parameters?
>
> But first, I would like to commit attached patch if ok? This time, 
> regression tests are green...

Yes, with one minor formatting nit:

+  }
+  else {

The else should be on the same line as the }.

  -Hal

>
> /Jonas
>

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory