getOperandsScalarizationOverhead()

Hal Finkel via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 24 06:08:52 PST 2017


On 02/24/2017 06:38 AM, Jonas Paulsson wrote:
> Hi Hal,
>
> >> while writing CostModel tests for SystemZ, I needed to fix 
> getOperandsScalarizationOverhead() so that it can handle vector 
> argument types.
> > Why? (A test case would help).
>
> Sorry I didn't mention it more clearly, but this patch will go in 
> together with the SystemZ cost functions patch, where the CostModel 
> tests have also been added. They don't work without this patch (bar 
> the check for Undef, which is just an improvement). CostModel will 
> make a call with vector arguments, which ends up here in common code. 
> https://reviews.llvm.org/D29631#fb86bef3

Ah, okay. FWIW, I'd prefer even small patches like this on Phabricator. 
When you commit this, please note in the commit message that the tests 
will come with the upcoming SystemZ commit. Also, I prefer asserts with 
messages (and without a space before the opening paren), how about:

+          assert (VF == A->getType()->getVectorNumElements());

becomes

+          assert(VF == A->getType()->getVectorNumElements() &&
                   "Argument vector type does not match VF");


otherwise, this LGTM.

  -Hal

>
> It would be great to get that patch approved (SystemZ parts are 
> already approved), and then commit these two together, but if you wish 
> I could perhaps merge this smaller patch into it?
>
>>
>> +        Type *VecTy = nullptr;
>>
>> Don't default initialize here (it is not needed).
>>
>> +        }
>> +        else
>>
>> else on the previous line with the }.
> fixed.
>
> Thanks,
>
> Jonas
>

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



More information about the llvm-commits mailing list