llvm dev - Sep 2018 - CallSiteBase::getCalledFunction and non-trivial calls

The immediate change I have in mind is in CallGraph; our implementation 
of LowerCall in AMDGPU currently relies on the the callee arguments 
being lowered before the call is lowered, and we simply do not support 
indirect calls. However, we should be able to support these bitcast 
calls, as they are effectively direct for our purposes, but the 
CallGraph does not seem to consider them (it uses .getCalledFunction()). 
Maybe a new function for 
`dyn_cast<Function>(CS.getCalledValue()->stripPointerCasts())` would 
make sense? I am not sure if this is valid to use in CallGraph, but it 
does not care about arguments as far as I can tell.

At the very least I can try to clarify the docs, because until you 
explained it I had a different definition of "indirect call" in mind.

Scott

On 2018-09-12 15:21, Friedman, Eli wrote:
> On 9/12/2018 11:51 AM, via llvm-dev wrote:
>> How does LLVM define "indirect call"? The documentation of 
>> CallSiteBase::getCalledFunction claims it returns null for indirect 
>> calls, but in practice it seems to return null for
"non-trivial"
>> calls. For example, it returns null for a direct call to a
bitcast'ed
>> function:
>> 
>> %0 = call void bitcast (void (%struct.foo *)* @func to void 
>> (%struct.bar *)*)(%struct.bar *qux)
>> 
>> By some definition "direct" could mean "trivial",
but here it seems
>> ambiguous at best.
> 
> An indirect call is a call to anything that isn't a Function.
> 
> It might be possible to argue for a special case for a bitcast of a
> Function, because the code generator will eventually look through the
> bitcast. But in practice transforms prefer to treat them as opaque
> anyway because they can't reason about the arguments.  And instcombine
> will transform code like your example into a direct ("trivial")
call.
> 
>> 
>> I was able to find some discussion of this previously at 
>> http://lists.llvm.org/pipermail/llvm-dev/2015-November/092396.html but 
>> it feels like the docs/implementation of some of the functions in 
>> CallSiteBase (isIndirectCall also does not seem quite correct, and is 
>> not used in a couple places it could be) should just be updated to 
>> reflect this.
>> 
>> Is my assessment reasonable? I can update these functions, but it will 
>> require updating uses throughout the codebase so I wanted to ask if 
>> this makes any sense before starting the work.
> 
> Documentation updates would be fine.  Not sure what you're proposing
> to change in terms of code...
> 
> -Eli

The current CallGraphSCC passes aren't prepared to deal with edges like 
that, I think, but that could be fixed; it's only a few passes.  Maybe 
simpler for your purposes to pull the call transform code out of 
instcombine so you can call it at -O0, though.

-Eli

On 9/12/2018 12:47 PM, scott at scottlinder.com wrote:
> The immediate change I have in mind is in CallGraph; our 
> implementation of LowerCall in AMDGPU currently relies on the the 
> callee arguments being lowered before the call is lowered, and we 
> simply do not support indirect calls. However, we should be able to 
> support these bitcast calls, as they are effectively direct for our 
> purposes, but the CallGraph does not seem to consider them (it uses 
> .getCalledFunction()). Maybe a new function for 
> `dyn_cast<Function>(CS.getCalledValue()->stripPointerCasts())`
would
> make sense? I am not sure if this is valid to use in CallGraph, but it 
> does not care about arguments as far as I can tell.
>
> At the very least I can try to clarify the docs, because until you 
> explained it I had a different definition of "indirect call" in
mind.
>
> Scott
>
> On 2018-09-12 15:21, Friedman, Eli wrote:
>> On 9/12/2018 11:51 AM, via llvm-dev wrote:
>>> How does LLVM define "indirect call"? The documentation
of
>>> CallSiteBase::getCalledFunction claims it returns null for indirect
>>> calls, but in practice it seems to return null for
"non-trivial"
>>> calls. For example, it returns null for a direct call to a 
>>> bitcast'ed function:
>>>
>>> %0 = call void bitcast (void (%struct.foo *)* @func to void 
>>> (%struct.bar *)*)(%struct.bar *qux)
>>>
>>> By some definition "direct" could mean
"trivial", but here it seems
>>> ambiguous at best.
>>
>> An indirect call is a call to anything that isn't a Function.
>>
>> It might be possible to argue for a special case for a bitcast of a
>> Function, because the code generator will eventually look through the
>> bitcast. But in practice transforms prefer to treat them as opaque
>> anyway because they can't reason about the arguments.  And
instcombine
>> will transform code like your example into a direct
("trivial") call.
>>
>>>
>>> I was able to find some discussion of this previously at 
>>> http://lists.llvm.org/pipermail/llvm-dev/2015-November/092396.html 
>>> but it feels like the docs/implementation of some of the functions 
>>> in CallSiteBase (isIndirectCall also does not seem quite correct, 
>>> and is not used in a couple places it could be) should just be 
>>> updated to reflect this.
>>>
>>> Is my assessment reasonable? I can update these functions, but it 
>>> will require updating uses throughout the codebase so I wanted to 
>>> ask if this makes any sense before starting the work.
>>
>> Documentation updates would be fine.  Not sure what you're
proposing
>> to change in terms of code...
>>
>> -Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project

It would be nice to have the new function that Scott suggests ... this
little detail has bitten us before, too. The current getCalledFunction()
creates an impression that we will always see the callee for a direct call,
but in reality it doesn't actually look through constant expressions. A new
function will make this more obvious to someone expecting to always find a
direct callee. Any use of this function will also make it apparent that the
analysis or pass is prepared to handle the arguments.

Sameer.

On Tue, Sep 18, 2018 at 1:54 AM, Friedman, Eli via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> The current CallGraphSCC passes aren't prepared to deal with edges like
> that, I think, but that could be fixed; it's only a few passes.  Maybe
> simpler for your purposes to pull the call transform code out of
> instcombine so you can call it at -O0, though.
>
> -Eli
>
> On 9/12/2018 12:47 PM, scott at scottlinder.com wrote:
>
>> The immediate change I have in mind is in CallGraph; our implementation
>> of LowerCall in AMDGPU currently relies on the the callee arguments
being
>> lowered before the call is lowered, and we simply do not support
indirect
>> calls. However, we should be able to support these bitcast calls, as
they
>> are effectively direct for our purposes, but the CallGraph does not
seem to
>> consider them (it uses .getCalledFunction()). Maybe a new function for
>> `dyn_cast<Function>(CS.getCalledValue()->stripPointerCasts())`
would
>> make sense? I am not sure if this is valid to use in CallGraph, but it
does
>> not care about arguments as far as I can tell.
>>
>> At the very least I can try to clarify the docs, because until you
>> explained it I had a different definition of "indirect call"
in mind.
>>
>> Scott
>>
>> On 2018-09-12 15:21, Friedman, Eli wrote:
>>
>>> On 9/12/2018 11:51 AM, via llvm-dev wrote:
>>>
>>>> How does LLVM define "indirect call"? The
documentation of
>>>> CallSiteBase::getCalledFunction claims it returns null for
indirect
>>>> calls, but in practice it seems to return null for
"non-trivial" calls. For
>>>> example, it returns null for a direct call to a bitcast'ed
function:
>>>>
>>>> %0 = call void bitcast (void (%struct.foo *)* @func to void
>>>> (%struct.bar *)*)(%struct.bar *qux)
>>>>
>>>> By some definition "direct" could mean
"trivial", but here it seems
>>>> ambiguous at best.
>>>>
>>>
>>> An indirect call is a call to anything that isn't a Function.
>>>
>>> It might be possible to argue for a special case for a bitcast of a
>>> Function, because the code generator will eventually look through
the
>>> bitcast. But in practice transforms prefer to treat them as opaque
>>> anyway because they can't reason about the arguments.  And
instcombine
>>> will transform code like your example into a direct
("trivial") call.
>>>
>>>
>>>> I was able to find some discussion of this previously at
>>>>
http://lists.llvm.org/pipermail/llvm-dev/2015-November/092396.html but
>>>> it feels like the docs/implementation of some of the functions
in
>>>> CallSiteBase (isIndirectCall also does not seem quite correct,
and is not
>>>> used in a couple places it could be) should just be updated to
reflect this.
>>>>
>>>> Is my assessment reasonable? I can update these functions, but
it will
>>>> require updating uses throughout the codebase so I wanted to
ask if this
>>>> makes any sense before starting the work.
>>>>
>>>
>>> Documentation updates would be fine.  Not sure what you're
proposing
>>> to change in terms of code...
>>>
>>> -Eli
>>>
>>
>
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
> Foundation Collaborative Project
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180918/92d95d28/attachment-0001.html>