[llvm] r218004 - Add a new pass FunctionTargetTransformInfo. This pass serves as a

[Eric Christopher]

Dave and I chatted about this for a bit...

On Wed, Sep 17, 2014 at 6:18 PM, Eric Christopher <echristo at gmail.com>
wrote:

>
>>> +  virtual const TargetSubtargetInfo *getSubtargetImpl(const Function *)
>>> const {
>>> +    return getSubtargetImpl();
>>>
>>
>> So what's left to do to make this actually do the real work of picking a
>> Subtarget based on the Function? I'm just curious about where that'll fit
>> in your migration plan?
>>
>>
> I was going to fix up all of the occurrences of getting a subtarget
> without a function first and then actually pick the function. That gives me
> a bunch of no functional change patches to get everything going.
>
>
>> Once this function is actually implemented, would it be possible to test
>> just this one change (& then test each subsequent feature as you port it
>> over)?
>>
>>
>
> There are no functional changes in this patch.
>
>

We've agreed that if I did the TargetMachine subtarget creation patch first
then there could be a visible change if I modified testcases to have a
default module and moved the cpu or target features down to the individual
functions in the test I could probably test this. It would involve a lot of
testcase modification and probable duplication to do this at the level of
function call granularity, i.e. test every API that goes through in a
separate way. I think we're going to compromise that I'll see what I can do
to add the TargetMachine subtarget creation stuff in now and then work on
testing functionality as I can.



> -  void getUnrollingPreferences(Loop *L,
>>> +  void getUnrollingPreferences(const Function *F, Loop *L,
>>>
>>
>> Would it make more sense for this to take the TargetSubtargetInfo
>> directly? A more constrained API (makes it clear what this function is
>> using - just the subtarget info, not other things with the Function) and
>> potentially fewer Subtarget lookups (which are zero cost today, but later
>> will become at least some kind of map lookup/lazy construction/etc?).
>>
>>
>
> Possibly. I thought about limiting it here, but the problem is that each
> particular target could have it's own desires and I was hoping for a more
> uniform API. It's definitely something that could be adjusted though.
>
>
>> -void PPCTTI::getUnrollingPreferences(Loop *L, UnrollingPreferences &UP)
>>> const {
>>> -  if (ST->getDarwinDirective() == PPC::DIR_A2) {
>>> +void PPCTTI::getUnrollingPreferences(const Function *F, Loop *L,
>>> +                                     UnrollingPreferences &UP) const {
>>>
>>
>> And in places like this, if you passed the Subtarget, you could even call
>> it "ST" and then the body of the function wouldn't need any changes at all.
>> (granted, shadowing is subtle stuff... so I can understand if that's not
>> ideal - but given the ultimate future will end up removing the member
>> variable, it might be an acceptable intermediate state to reduce
>> typing/churn)
>>
>
> Right. And this would be why it might work. I.e. instead of Function, take
> Subtarget since that'll be what everything uses ultimately to get the
> various target data structures. I have no ultimate preference here other
> than I will likely have to cast that subtarget no matter what to be the
> type of the actual TargetSubtargetInfo that the target wants (I.e.
> PPCSubtargetInfo).
>
> That said, it doesn't necessarily lose me anything.
>

Having thought about this I think I'm going to go with it. I'll still have
to do the specific casts but it will save some additional lookups and
compile time. It might be premature optimization, but the API isn't any
worse with it so let's do it.

-eric

>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140917/5e1648fc/attachment.html>