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

[Duncan P. N. Exon Smith]

> On 2015-Jul-28, at 17:18, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> 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.

Thanks for explaining!