[llvm] r225254 - InstCombine: Bitcast call arguments from/to pointer/integer type
David Majnemer
david.majnemer at gmail.com
Wed Mar 11 11:05:56 PDT 2015
Fixed in r231948.
On Wed, Mar 11, 2015 at 10:16 AM, Steven Wu <stevenwu at apple.com> wrote:
> Hi David
>
> I found a problem introduced by this patch. We don’t respect byval
> attribute on the function parameter when we instcombine function cast.
> Attach a testcase.
> In the testcase, test_byval function has a parameter defined as pass
> byval, but at the callsite, it does the argument expansion.
> Without instcombine, they produces compatible interface. After
> instcombine, now test_byval treats the first field in the struct as the
> pointer and everything fall apart.
> This optimization used to be stopped by isBitCastable, but now the data
> layout says it is ok to cast a pointer to i32.
> Since we actually check the byval attribute on the argument, it is weird
> that we didn’t do the same check for parameter. If you think this is the
> right fix, I will prepare a patch for it.
>
> Thanks
>
> Steven
>
>
>
> > On Jan 6, 2015, at 12:41 AM, David Majnemer <david.majnemer at gmail.com>
> wrote:
> >
> > Author: majnemer
> > Date: Tue Jan 6 02:41:31 2015
> > New Revision: 225254
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=225254&view=rev
> > Log:
> > InstCombine: Bitcast call arguments from/to pointer/integer type
> >
> > Try harder to get rid of bitcast'd calls by ptrtoint/inttoptr'ing
> > arguments and return values when DataLayout says it is safe to do so.
> >
> > Modified:
> > llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
> > llvm/trunk/test/Transforms/InstCombine/apint-call-cast-target.ll
> > llvm/trunk/test/Transforms/InstCombine/call-cast-target.ll
> >
> > Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp?rev=225254&r1=225253&r2=225254&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp (original)
> > +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp Tue Jan
> 6 02:41:31 2015
> > @@ -1416,7 +1416,7 @@ bool InstCombiner::transformConstExprCas
> > if (NewRetTy->isStructTy())
> > return false; // TODO: Handle multiple return values.
> >
> > - if (!CastInst::isBitCastable(NewRetTy, OldRetTy)) {
> > + if (!CastInst::isBitOrNoopPointerCastable(NewRetTy, OldRetTy, DL)) {
> > if (Callee->isDeclaration())
> > return false; // Cannot transform this return value.
> >
> > @@ -1451,12 +1451,21 @@ bool InstCombiner::transformConstExprCas
> > unsigned NumActualArgs = CS.arg_size();
> > unsigned NumCommonArgs = std::min(FT->getNumParams(), NumActualArgs);
> >
> > + // Prevent us turning:
> > + // declare void @takes_i32_inalloca(i32* inalloca)
> > + // call void bitcast (void (i32*)* @takes_i32_inalloca to void
> (i32)*)(i32 0)
> > + //
> > + // into:
> > + // call void @takes_i32_inalloca(i32* null)
> > + if (Callee->getAttributes().hasAttrSomewhere(Attribute::InAlloca))
> > + return false;
> > +
> > CallSite::arg_iterator AI = CS.arg_begin();
> > for (unsigned i = 0, e = NumCommonArgs; i != e; ++i, ++AI) {
> > 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 +1560,7 @@ bool InstCombiner::transformConstExprCas
> > 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.
> > @@ -1622,7 +1631,7 @@ bool InstCombiner::transformConstExprCas
> > Value *NV = NC;
> > if (OldRetTy != NV->getType() && !Caller->use_empty()) {
> > if (!NV->getType()->isVoidTy()) {
> > - NV = NC = CastInst::Create(CastInst::BitCast, NC, OldRetTy);
> > + NV = NC = CastInst::CreateBitOrPointerCast(NC, OldRetTy);
> > NC->setDebugLoc(Caller->getDebugLoc());
> >
> > // If this is an invoke instruction, we should insert it after the
> first
> >
> > Modified:
> llvm/trunk/test/Transforms/InstCombine/apint-call-cast-target.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/apint-call-cast-target.ll?rev=225254&r1=225253&r2=225254&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/test/Transforms/InstCombine/apint-call-cast-target.ll
> (original)
> > +++ llvm/trunk/test/Transforms/InstCombine/apint-call-cast-target.ll Tue
> Jan 6 02:41:31 2015
> > @@ -5,15 +5,18 @@ target triple = "i686-pc-linux-gnu"
> >
> > define i32 @main() {
> > ; CHECK-LABEL: @main(
> > -; CHECK: call i32 bitcast
> > +; CHECK: %[[call:.*]] = call i7* @ctime(i999* null)
> > +; CHECK: %[[cast:.*]] = ptrtoint i7* %[[call]] to i32
> > +; CHECK: ret i32 %[[cast]]
> > entry:
> > %tmp = call i32 bitcast (i7* (i999*)* @ctime to i32 (i99*)*)( i99*
> null )
> > ret i32 %tmp
> > }
> >
> > define i7* @ctime(i999*) {
> > -; CHECK-LABEL: @ctime(
> > -; CHECK: call i7* bitcast
> > +; CHECK-LABEL: define i7* @ctime(
> > +; CHECK: %[[call:.*]] = call i32 @main()
> > +; CHECK: %[[cast:.*]] = inttoptr i32 %[[call]] to i7*
> > entry:
> > %tmp = call i7* bitcast (i32 ()* @main to i7* ()*)( )
> > ret i7* %tmp
> >
> > Modified: llvm/trunk/test/Transforms/InstCombine/call-cast-target.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/call-cast-target.ll?rev=225254&r1=225253&r2=225254&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/test/Transforms/InstCombine/call-cast-target.ll (original)
> > +++ llvm/trunk/test/Transforms/InstCombine/call-cast-target.ll Tue Jan
> 6 02:41:31 2015
> > @@ -5,7 +5,9 @@ target triple = "i686-pc-linux-gnu"
> >
> > define i32 @main() {
> > ; CHECK-LABEL: @main
> > -; CHECK: call i32 bitcast
> > +; CHECK: %[[call:.*]] = call i8* @ctime(i32* null)
> > +; CHECK: %[[cast:.*]] = ptrtoint i8* %[[call]] to i32
> > +; CHECK: ret i32 %[[cast]]
> > entry:
> > %tmp = call i32 bitcast (i8* (i32*)* @ctime to i32 (i32*)*)( i32* null
> ) ; <i32> [#uses=1]
> > ret i32 %tmp
> > @@ -25,3 +27,37 @@ entry:
> > %0 = call { i8 } bitcast ({ i8 } (i32*)* @foo to { i8 } (i16*)*)(i16*
> null)
> > ret void
> > }
> > +
> > +declare i32 @fn1(i32)
> > +
> > +define i32 @test1(i32* %a) {
> > +; CHECK-LABEL: @test1
> > +; CHECK: %[[cast:.*]] = ptrtoint i32* %a to i32
> > +; CHECK-NEXT: %[[call:.*]] = tail call i32 @fn1(i32 %[[cast]])
> > +; CHECK-NEXT: ret i32 %[[call]]
> > +entry:
> > + %call = tail call i32 bitcast (i32 (i32)* @fn1 to i32 (i32*)*)(i32*
> %a)
> > + ret i32 %call
> > +}
> > +
> > +declare i32 @fn2(i16)
> > +
> > +define i32 @test2(i32* %a) {
> > +; CHECK-LABEL: @test2
> > +; CHECK: %[[call:.*]] = tail call i32 bitcast (i32 (i16)* @fn2 to
> i32 (i32*)*)(i32* %a)
> > +; CHECK-NEXT: ret i32 %[[call]]
> > +entry:
> > + %call = tail call i32 bitcast (i32 (i16)* @fn2 to i32 (i32*)*)(i32*
> %a)
> > + ret i32 %call
> > +}
> > +
> > +declare i32 @fn3(i64)
> > +
> > +define i32 @test3(i32* %a) {
> > +; CHECK-LABEL: @test3
> > +; CHECK: %[[call:.*]] = tail call i32 bitcast (i32 (i64)* @fn3 to
> i32 (i32*)*)(i32* %a)
> > +; CHECK-NEXT: ret i32 %[[call]]
> > +entry:
> > + %call = tail call i32 bitcast (i32 (i64)* @fn3 to i32 (i32*)*)(i32*
> %a)
> > + ret i32 %call
> > +}
> >
> >
> > _______________________________________________
> > 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/20150311/cc9fa3f1/attachment.html>
More information about the llvm-commits
mailing list