<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jul 28, 2015 at 3:58 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><br>
> On 2015-Jul-27, at 16:32, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
> Author: dblaikie<br>
> Date: Mon Jul 27 18:32:19 2015<br>
> New Revision: 243349<br>
><br>
> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D243349-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=GsaOnSLce8ruJZCkdCteqFh7M7sJGRJJsOc3F75XavU&s=i1i8UwGRUazEFqZmab-RJxbnKMWISfT8OJehnZ4dqZA&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=243349&view=rev</a><br>
> Log:<br>
> [opaque pointers] Avoid the use of pointee types when parsing inline asm in IR<br>
><br>
> When parsing calls to inline asm the pointee type (of the pointer type<br>
> representing the value type of the InlineAsm value) was used. To avoid<br>
> using it, use the ValID structure to ferry the FunctionType directly<br>
> through to the InlineAsm construction.<br>
><br>
> This is a bit of a workaround - alternatively the inline asm could<br>
> explicitly describe the type but that'd be verbose/redundant in the IR<br>
> and so long as the inline asm calls directly in the context of a call or<br>
> invoke, this should suffice.<br>
><br>
> Modified:<br>
>    llvm/trunk/lib/AsmParser/LLParser.cpp<br>
>    llvm/trunk/lib/AsmParser/LLParser.h<br>
><br>
> Modified: llvm/trunk/lib/AsmParser/LLParser.cpp<br>
> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_AsmParser_LLParser.cpp-3Frev-3D243349-26r1-3D243348-26r2-3D243349-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=GsaOnSLce8ruJZCkdCteqFh7M7sJGRJJsOc3F75XavU&s=xVO169F-mkFHdbfNUbFK2U3LbaXr5GmWyQb2VPPrExs&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.cpp?rev=243349&r1=243348&r2=243349&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/AsmParser/LLParser.cpp (original)<br>
> +++ llvm/trunk/lib/AsmParser/LLParser.cpp Mon Jul 27 18:32:19 2015<br>
> @@ -3980,13 +3980,12 @@ bool LLParser::ConvertValIDToValue(Type<br>
>     V = PFS->GetVal(ID.StrVal, Ty, ID.Loc);<br>
>     return V == nullptr;<br>
>   case ValID::t_InlineAsm: {<br>
> -    PointerType *PTy = dyn_cast<PointerType>(Ty);<br>
> -    FunctionType *FTy =<br>
> -      PTy ? dyn_cast<FunctionType>(PTy->getElementType()) : nullptr;<br>
> -    if (!FTy || !InlineAsm::Verify(FTy, ID.StrVal2))<br>
> +    assert(ID.FTy);<br>
<br>
</div></div>It's not clear to me why this should be an assert instead of an Error<br>
after your code change, unless it should have been an assert before?<br>
Was it already guaranteed?<br></blockquote><div><br>Thanks for taking a look!<br><br></div><div>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.<br><br>- David</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
> +    if (!InlineAsm::Verify(ID.FTy, ID.StrVal2))<br>
>       return Error(ID.Loc, "invalid type for inline asm constraint string");<br>
> -    V = InlineAsm::get(FTy, ID.StrVal, ID.StrVal2, ID.UIntVal&1,<br>
> -                       (ID.UIntVal>>1)&1, (InlineAsm::AsmDialect(ID.UIntVal>>2)));<br>
> +    V = InlineAsm::get(ID.FTy, ID.StrVal, ID.StrVal2, ID.UIntVal & 1,<br>
> +                       (ID.UIntVal >> 1) & 1,<br>
> +                       (InlineAsm::AsmDialect(ID.UIntVal >> 2)));<br>
>     return false;<br>
>   }<br>
>   case ValID::t_GlobalName:<br>
> @@ -4864,6 +4863,8 @@ bool LLParser::ParseInvoke(Instruction *<br>
>     Ty = FunctionType::get(RetType, ParamTypes, false);<br>
>   }<br>
><br>
> +  CalleeID.FTy = Ty;<br>
> +<br>
>   // Look up the callee.<br>
>   Value *Callee;<br>
>   if (ConvertValIDToValue(PointerType::getUnqual(Ty), CalleeID, Callee, &PFS))<br>
> @@ -5277,6 +5278,8 @@ bool LLParser::ParseCall(Instruction *&I<br>
>     Ty = FunctionType::get(RetType, ParamTypes, false);<br>
>   }<br>
><br>
> +  CalleeID.FTy = Ty;<br>
> +<br>
>   // Look up the callee.<br>
>   Value *Callee;<br>
>   if (ConvertValIDToValue(PointerType::getUnqual(Ty), CalleeID, Callee, &PFS))<br>
><br>
> Modified: llvm/trunk/lib/AsmParser/LLParser.h<br>
> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_AsmParser_LLParser.h-3Frev-3D243349-26r1-3D243348-26r2-3D243349-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=GsaOnSLce8ruJZCkdCteqFh7M7sJGRJJsOc3F75XavU&s=kLNtOUVynQwhorFPWV0gDhr54cVvx0Mzj55Vm9UtqAA&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.h?rev=243349&r1=243348&r2=243349&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/AsmParser/LLParser.h (original)<br>
> +++ llvm/trunk/lib/AsmParser/LLParser.h Mon Jul 27 18:32:19 2015<br>
> @@ -52,13 +52,14 @@ namespace llvm {<br>
>       t_Null, t_Undef, t_Zero,    // No value.<br>
>       t_EmptyArray,               // No value:  []<br>
>       t_Constant,                 // Value in ConstantVal.<br>
> -      t_InlineAsm,                // Value in StrVal/StrVal2/UIntVal.<br>
> +      t_InlineAsm,                // Value in FTy/StrVal/StrVal2/UIntVal.<br>
>       t_ConstantStruct,           // Value in ConstantStructElts.<br>
>       t_PackedConstantStruct      // Value in ConstantStructElts.<br>
>     } Kind;<br>
><br>
>     LLLexer::LocTy Loc;<br>
>     unsigned UIntVal;<br>
> +    FunctionType *FTy;<br>
>     std::string StrVal, StrVal2;<br>
>     APSInt APSIntVal;<br>
>     APFloat APFloatVal;<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
</div></div></blockquote></div><br></div></div>