[llvm] r243349 - [opaque pointers] Avoid the use of pointee types when parsing inline asm in IR

[David Blaikie]

On Tue, Jul 28, 2015 at 3:58 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

>
> > On 2015-Jul-27, at 16:32, David Blaikie <dblaikie at gmail.com> wrote:
> >
> > Author: dblaikie
> > Date: Mon Jul 27 18:32:19 2015
> > New Revision: 243349
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=243349&view=rev
> > Log:
> > [opaque pointers] Avoid the use of pointee types when parsing inline asm
> in IR
> >
> > When parsing calls to inline asm the pointee type (of the pointer type
> > representing the value type of the InlineAsm value) was used. To avoid
> > using it, use the ValID structure to ferry the FunctionType directly
> > through to the InlineAsm construction.
> >
> > This is a bit of a workaround - alternatively the inline asm could
> > explicitly describe the type but that'd be verbose/redundant in the IR
> > and so long as the inline asm calls directly in the context of a call or
> > invoke, this should suffice.
> >
> > Modified:
> >    llvm/trunk/lib/AsmParser/LLParser.cpp
> >    llvm/trunk/lib/AsmParser/LLParser.h
> >
> > Modified: llvm/trunk/lib/AsmParser/LLParser.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.cpp?rev=243349&r1=243348&r2=243349&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/AsmParser/LLParser.cpp (original)
> > +++ llvm/trunk/lib/AsmParser/LLParser.cpp Mon Jul 27 18:32:19 2015
> > @@ -3980,13 +3980,12 @@ bool LLParser::ConvertValIDToValue(Type
> >     V = PFS->GetVal(ID.StrVal, Ty, ID.Loc);
> >     return V == nullptr;
> >   case ValID::t_InlineAsm: {
> > -    PointerType *PTy = dyn_cast<PointerType>(Ty);
> > -    FunctionType *FTy =
> > -      PTy ? dyn_cast<FunctionType>(PTy->getElementType()) : nullptr;
> > -    if (!FTy || !InlineAsm::Verify(FTy, ID.StrVal2))
> > +    assert(ID.FTy);
>
> It's not clear to me why this should be an assert instead of an Error
> after your code change, unless it should have been an assert before?
> Was it already guaranteed?
>

Thanks for taking a look!

Sorry, a little subtle - previously the code was using the passed in Ty,
now it's using FTy that's squirreled away in the ID. All callers (I only
know of the two - CreateCall and CreateInvoke) should've set FTy on the ID
already. If a caller hasn't, that's a bug in the AsmParser I need to fix -
not a bug in the input.

- David


>
> > +    if (!InlineAsm::Verify(ID.FTy, ID.StrVal2))
> >       return Error(ID.Loc, "invalid type for inline asm constraint
> string");
> > -    V = InlineAsm::get(FTy, ID.StrVal, ID.StrVal2, ID.UIntVal&1,
> > -                       (ID.UIntVal>>1)&1,
> (InlineAsm::AsmDialect(ID.UIntVal>>2)));
> > +    V = InlineAsm::get(ID.FTy, ID.StrVal, ID.StrVal2, ID.UIntVal & 1,
> > +                       (ID.UIntVal >> 1) & 1,
> > +                       (InlineAsm::AsmDialect(ID.UIntVal >> 2)));
> >     return false;
> >   }
> >   case ValID::t_GlobalName:
> > @@ -4864,6 +4863,8 @@ bool LLParser::ParseInvoke(Instruction *
> >     Ty = FunctionType::get(RetType, ParamTypes, false);
> >   }
> >
> > +  CalleeID.FTy = Ty;
> > +
> >   // Look up the callee.
> >   Value *Callee;
> >   if (ConvertValIDToValue(PointerType::getUnqual(Ty), CalleeID, Callee,
> &PFS))
> > @@ -5277,6 +5278,8 @@ bool LLParser::ParseCall(Instruction *&I
> >     Ty = FunctionType::get(RetType, ParamTypes, false);
> >   }
> >
> > +  CalleeID.FTy = Ty;
> > +
> >   // Look up the callee.
> >   Value *Callee;
> >   if (ConvertValIDToValue(PointerType::getUnqual(Ty), CalleeID, Callee,
> &PFS))
> >
> > Modified: llvm/trunk/lib/AsmParser/LLParser.h
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.h?rev=243349&r1=243348&r2=243349&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/AsmParser/LLParser.h (original)
> > +++ llvm/trunk/lib/AsmParser/LLParser.h Mon Jul 27 18:32:19 2015
> > @@ -52,13 +52,14 @@ namespace llvm {
> >       t_Null, t_Undef, t_Zero,    // No value.
> >       t_EmptyArray,               // No value:  []
> >       t_Constant,                 // Value in ConstantVal.
> > -      t_InlineAsm,                // Value in StrVal/StrVal2/UIntVal.
> > +      t_InlineAsm,                // Value in
> FTy/StrVal/StrVal2/UIntVal.
> >       t_ConstantStruct,           // Value in ConstantStructElts.
> >       t_PackedConstantStruct      // Value in ConstantStructElts.
> >     } Kind;
> >
> >     LLLexer::LocTy Loc;
> >     unsigned UIntVal;
> > +    FunctionType *FTy;
> >     std::string StrVal, StrVal2;
> >     APSInt APSIntVal;
> >     APFloat APFloatVal;
> >
> >
> > _______________________________________________
> > 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/20150728/27f35e81/attachment.html>