[llvm] r241220 - Add a routine to TargetTransformInfo that will allow targets to look
Eric Christopher
echristo at gmail.com
Wed Jul 29 15:11:26 PDT 2015
On Mon, Jul 27, 2015 at 10:36 AM David Blaikie <dblaikie at gmail.com> wrote:
> On Wed, Jul 1, 2015 at 6:42 PM, Eric Christopher <echristo at gmail.com>
> wrote:
>
>>
>>
>> On Wed, Jul 1, 2015 at 6:38 PM David Blaikie <dblaikie at gmail.com> wrote:
>>
>>> On Wed, Jul 1, 2015 at 6:28 PM, Eric Christopher <echristo at gmail.com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Wed, Jul 1, 2015 at 6:26 PM David Blaikie <dblaikie at gmail.com>
>>>> wrote:
>>>>
>>>>> On Wed, Jul 1, 2015 at 6:11 PM, Eric Christopher <echristo at gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Author: echristo
>>>>>> Date: Wed Jul 1 20:11:47 2015
>>>>>> New Revision: 241220
>>>>>>
>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=241220&view=rev
>>>>>> Log:
>>>>>> Add a routine to TargetTransformInfo that will allow targets to look
>>>>>> at the attributes on a function to determine whether or not to allow
>>>>>> inlining.
>>>>>>
>>>>>> Modified:
>>>>>> llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h
>>>>>> llvm/trunk/include/llvm/Analysis/TargetTransformInfoImpl.h
>>>>>> llvm/trunk/lib/Analysis/IPA/InlineCost.cpp
>>>>>> llvm/trunk/lib/Analysis/TargetTransformInfo.cpp
>>>>>>
>>>>>> Modified: llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h?rev=241220&r1=241219&r2=241220&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h (original)
>>>>>> +++ llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h Wed Jul 1
>>>>>> 20:11:47 2015
>>>>>> @@ -519,6 +519,11 @@ public:
>>>>>> Value *getOrCreateResultFromMemIntrinsic(IntrinsicInst *Inst,
>>>>>> Type *ExpectedType) const;
>>>>>>
>>>>>> + /// \returns True if the two functions have compatible attributes
>>>>>> for inlining
>>>>>> + /// purposes.
>>>>>> + bool hasCompatibleFunctionAttributes(const Function *Caller,
>>>>>> + const Function *Callee) const;
>>>>>>
>>>>>
>>>>> Given that it gets the whole function, perhaps it should be
>>>>> "areCompatibleFunctions"? (they could look at other properties of the
>>>>> function)
>>>>>
>>>>>
>>>> Naming is hard. No particular preference here so if you like this one
>>>> I'll happily change it :)
>>>>
>>>
>>> Right - well, while we're discussing it, is "compatible" fairly
>>> unambiguous here - I would imagine there are many senses in which two
>>> functions may be compatible? I'm wondering if it would be helpful to
>>> clarify what kind of compatibility this test is about. Maybe even
>>> "isInlinable"
>>>
>>
>> Yeah. I was trying to make it fairly specific without having a 40
>> character name for the function. :\
>>
>> I'll take any and all naming help :)
>>
>
> isInlineCompatible/areInlineCompatible? (that it's talking about
> compatible Functions is implied by the type of the operands - but if it
> helps to add/keep that word in, I don't think it does much harm/makes it
> too verbose)
>
>
areInlineCompatible seems pretty good, let's go with that.
Done thusly:
M include/llvm/Analysis/TargetTransformInfo.h
M include/llvm/Analysis/TargetTransformInfoImpl.h
M lib/Analysis/TargetTransformInfo.cpp
M lib/Analysis/IPA/InlineCost.cpp
M lib/Target/X86/X86TargetTransformInfo.cpp
M lib/Target/X86/X86TargetTransformInfo.h
r243578 = 53bb3690e155496f530f5f93b94c2c3cee387520
(refs/remotes/origin/master)
and thanks :)
-eric
> Is this symmetric? I assume not - so maybe it could be more explicit about
> that - isInlineable?
>
> (any of these names don't imply anything about this function only doing
> the machine-specific parts of the inline test (presumably there's lots of
> generic cases that other code tests - recursion, setjmp/longjmp or
> whatever) but the context of this being a member function of
> TargetTransform I guess implies that sufficientnly?)
>
>
>> Otherwise, can the attribute set/list/container be passed directly, not
>>>>> exposing the whole function to TTI?
>>>>>
>>>>>
>>>>
>>>> They could look at other properties, also if you take a look at the x86
>>>> one it makes it a bit more obvious that I'm using the function to get
>>>> parsed subtarget attributes there.
>>>>
>>>
>>> Right - but other implementations (or this one) might look at other
>>> properties eventually. Or it might be useful to be able to just mock up
>>> some subtarget attributes (perhaps the subtarget attributes of some other
>>> function - not the caller, but a potential caller (function splitting? "I'm
>>> considering splitting out half of this function & want to see if the
>>> attributes would be OK for inlining this call - but don't look at the rest
>>> of the original function, it's not entirely important")).
>>>
>>> But yeah, no idea what's appropriate/useful/sufficient in TTI, I haven't
>>> looked at its API or have much of a sense of how things are done.
>>> +Chandler, since he's tangentially involved given the inliner work, in case
>>> any of what I'm rambling about makes sense to him. Otherwise we'll just
>>> assume I lack sufficiently useful context here.
>>>
>>>
>> Oh, no I was saying that we didn't want to just pass in the bits etc
>> because we might want to look at other things - agreeing with you :)
>>
>> -eric
>>
>>
>>> - Dave
>>>
>>>
>>>>
>>>> -eric
>>>>
>>>>
>>>>> +
>>>>>> /// @}
>>>>>>
>>>>>> private:
>>>>>> @@ -619,6 +624,8 @@ public:
>>>>>> MemIntrinsicInfo &Info) = 0;
>>>>>> virtual Value *getOrCreateResultFromMemIntrinsic(IntrinsicInst
>>>>>> *Inst,
>>>>>> Type
>>>>>> *ExpectedType) = 0;
>>>>>> + virtual bool hasCompatibleFunctionAttributes(const Function
>>>>>> *Caller,
>>>>>> + const Function
>>>>>> *Callee) const = 0;
>>>>>> };
>>>>>>
>>>>>> template <typename T>
>>>>>> @@ -804,6 +811,10 @@ public:
>>>>>> Type *ExpectedType)
>>>>>> override {
>>>>>> return Impl.getOrCreateResultFromMemIntrinsic(Inst,
>>>>>> ExpectedType);
>>>>>> }
>>>>>> + bool hasCompatibleFunctionAttributes(const Function *Caller,
>>>>>> + const Function *Callee) const
>>>>>> override {
>>>>>> + return Impl.hasCompatibleFunctionAttributes(Caller, Callee);
>>>>>> + }
>>>>>> };
>>>>>>
>>>>>> template <typename T>
>>>>>>
>>>>>> Modified: llvm/trunk/include/llvm/Analysis/TargetTransformInfoImpl.h
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/TargetTransformInfoImpl.h?rev=241220&r1=241219&r2=241220&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- llvm/trunk/include/llvm/Analysis/TargetTransformInfoImpl.h
>>>>>> (original)
>>>>>> +++ llvm/trunk/include/llvm/Analysis/TargetTransformInfoImpl.h Wed
>>>>>> Jul 1 20:11:47 2015
>>>>>> @@ -335,6 +335,14 @@ public:
>>>>>> Type *ExpectedType) {
>>>>>> return nullptr;
>>>>>> }
>>>>>> +
>>>>>> + bool hasCompatibleFunctionAttributes(const Function *Caller,
>>>>>> + const Function *Callee) const
>>>>>> {
>>>>>> + return (Caller->getFnAttribute("target-cpu") ==
>>>>>> + Callee->getFnAttribute("target-cpu")) &&
>>>>>> + (Caller->getFnAttribute("target-features") ==
>>>>>> + Callee->getFnAttribute("target-features"));
>>>>>> + }
>>>>>> };
>>>>>>
>>>>>> /// \brief CRTP base class for use as a mix-in that aids implementing
>>>>>>
>>>>>> Modified: llvm/trunk/lib/Analysis/IPA/InlineCost.cpp
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/IPA/InlineCost.cpp?rev=241220&r1=241219&r2=241220&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- llvm/trunk/lib/Analysis/IPA/InlineCost.cpp (original)
>>>>>> +++ llvm/trunk/lib/Analysis/IPA/InlineCost.cpp Wed Jul 1 20:11:47
>>>>>> 2015
>>>>>> @@ -1344,9 +1344,9 @@ static bool attributeMatches(Function *F
>>>>>> /// \brief Test that there are no attribute conflicts between Caller
>>>>>> and Callee
>>>>>> /// that prevent inlining.
>>>>>> static bool functionsHaveCompatibleAttributes(Function *Caller,
>>>>>> - Function *Callee) {
>>>>>> - return attributeMatches(Caller, Callee, "target-cpu") &&
>>>>>> - attributeMatches(Caller, Callee, "target-features") &&
>>>>>> + Function *Callee,
>>>>>> + TargetTransformInfo
>>>>>> &TTI) {
>>>>>> + return TTI.hasCompatibleFunctionAttributes(Caller, Callee) &&
>>>>>> attributeMatches(Caller, Callee,
>>>>>> Attribute::SanitizeAddress) &&
>>>>>> attributeMatches(Caller, Callee, Attribute::SanitizeMemory)
>>>>>> &&
>>>>>> attributeMatches(Caller, Callee, Attribute::SanitizeThread);
>>>>>> @@ -1368,7 +1368,8 @@ InlineCost InlineCostAnalysis::getInline
>>>>>>
>>>>>> // Never inline functions with conflicting attributes (unless
>>>>>> callee has
>>>>>> // always-inline attribute).
>>>>>> - if (!functionsHaveCompatibleAttributes(CS.getCaller(), Callee))
>>>>>> + if (!functionsHaveCompatibleAttributes(CS.getCaller(), Callee,
>>>>>> + TTIWP->getTTI(*Callee)))
>>>>>> return llvm::InlineCost::getNever();
>>>>>>
>>>>>> // Don't inline this call if the caller has the optnone attribute.
>>>>>>
>>>>>> Modified: llvm/trunk/lib/Analysis/TargetTransformInfo.cpp
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/TargetTransformInfo.cpp?rev=241220&r1=241219&r2=241220&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- llvm/trunk/lib/Analysis/TargetTransformInfo.cpp (original)
>>>>>> +++ llvm/trunk/lib/Analysis/TargetTransformInfo.cpp Wed Jul 1
>>>>>> 20:11:47 2015
>>>>>> @@ -284,6 +284,11 @@ Value *TargetTransformInfo::getOrCreateR
>>>>>> return TTIImpl->getOrCreateResultFromMemIntrinsic(Inst,
>>>>>> ExpectedType);
>>>>>> }
>>>>>>
>>>>>> +bool TargetTransformInfo::hasCompatibleFunctionAttributes(
>>>>>> + const Function *Caller, const Function *Callee) const {
>>>>>> + return TTIImpl->hasCompatibleFunctionAttributes(Caller, Callee);
>>>>>> +}
>>>>>> +
>>>>>> TargetTransformInfo::Concept::~Concept() {}
>>>>>>
>>>>>> TargetIRAnalysis::TargetIRAnalysis() : TTICallback(&getDefaultTTI) {}
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> llvm-commits mailing list
>>>>>> llvm-commits at cs.uiuc.edu
>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>>>
>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150729/89d777e6/attachment.html>
More information about the llvm-commits
mailing list