[LLVMdev] should AlwaysInliner inline this case?

[David Majnemer]

On Mon, Jan 5, 2015 at 1:40 AM, Pete Cooper <peter_cooper at apple.com> wrote:

> Hi lx, Philip
>
> I've seen an instcombine which helps with this situation. It fires when
> the function types on both sides of the bitcast have the same number of
> operands and compatible types. It then adds bitcasts on the arguments and
> removes the one on the called function.
>

It indeed does, InstCombiner::transformConstExprCastCall.


>
> I don't have IR to hand, but it would be worth passing your IR through
> instcombine to see if that helps you.
>

The following should be a sufficiently workable example of what we would
hope to transform.

target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

define i32 @g(i32* %a) {
entry:
  %call = tail call i32 bitcast (i32 (i64)* @f to i32 (i32*)*)(i32* %a)
  ret i32 %call
}

declare i32 @f(i64)

define i32 @h(i64 %a) {
entry:
  %call = tail call i32 bitcast (i32 (i32*)* @g to i32 (i64)*)(i64 %a)
  ret i32 %call
}


>
> The idea of improving the inliner is also great, but you may find that
> it's needed for cases other than this one if i'm right about the
> instcombine.
>

Sadly, the combine fails because InstCombine
queries CastInst::isBitCastable to determine the castable-ness of the
parameter type and the argument type.  It isn't bitcastable though, it's
ptrtoint/inttoptr castable.

The following patch opens up the optimization:
--- a/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -1456,7 +1456,7 @@ bool
InstCombiner::transformConstExprCastCall(CallSite CS) {
     Type *ParamTy = FT->getParamType(i);
     Type *ActTy = (*AI)->getType();

-    if (!CastInst::isBitCastable(ActTy, ParamTy))
+    if (!CastInst::isBitOrNoopPointerCastable(ActTy, ParamTy, DL))
       return false;   // Cannot transform this parameter value.

     if (AttrBuilder(CallerPAL.getParamAttributes(i + 1), i + 1).
@@ -1551,7 +1551,7 @@ bool
InstCombiner::transformConstExprCastCall(CallSite CS) {
     if ((*AI)->getType() == ParamTy) {
       Args.push_back(*AI);
     } else {
-      Args.push_back(Builder->CreateBitCast(*AI, ParamTy));
+      Args.push_back(Builder->CreateBitOrPointerCast(*AI, ParamTy));
     }

     // Add any parameter attributes.

Running opt -instcombine -inline -instcombine with this patch results in:
define i32 @g(i32* %a) {
entry:
  %0 = ptrtoint i32* %a to i64
  %call = tail call i32 @f(i64 %0)
  ret i32 %call
}

declare i32 @f(i64)

define i32 @h(i64 %a) {
entry:
  %call.i = tail call i32 @f(i64 %a)
  ret i32 %call.i
}


>
> Thanks
> Pete
>
> Sent from my iPhone
>
> On Jan 5, 2015, at 3:16 AM, Liu Xin <navy.xliu at gmail.com> wrote:
>
> Philip,
>
> I post here because I think AlwaysInliner should inline it. I want to
> detect the indirect calls for Inliner, and I want to hear inputs.
>
> let me define indirect call first in my idea. In one single expression,
> one function may be subject to bitcast more than one time. we can detect
> this situation and treat it as a regular call of last function, is that
> okay?
>
> thanks,
> --lx
>
>
> On Mon, Jan 5, 2015 at 7:32 AM, Philip Reames <listmail at philipreames.com>
> wrote:
>
>> On 01/04/2015 12:04 AM, Liu Xin wrote:
>>
>>>
>>> %294= callfloatbitcast (float(float, float*)* @__gpu_modff to
>>> float(float, i64)*)(float%293, i64 %preg.212.addr.0)
>>>
>>> as you may know, some gpu backends don't support function call. we need
>>> to make sure to inline all functions here. however, Inliner can not figure
>>> out that this is a valid callsite in this form. actually, it is.  in C
>>> words, cast a function and then call should be treat as callsite, right?
>>>
>>>  Generally, the inliner doesn't do much with indirect calls, but given
>> there is no simpler canonical case here, I expect we'll have to.
>>
>> Its possible we might even want to define this as a direct call. I'm not
>> sure what the expectations are with regards to the type of the function
>> being called and the type of the callsite.  I suspect a lot of code would
>> get confused if getCalledFunction returned __gpu_modff with it's unbitcast
>> type.  That's possibly something we should fix though.
>>
>> We'll want to get other folks input here, but a small patch to the
>> inliner to handle this case would seem reasonable to me.
>>
>> Philip
>>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150105/07d3bc01/attachment.html>