[llvm-commits] [llvm-gcc] Intrinsics.patch (please apply)
Chris Lattner
clattner at apple.com
Thu May 31 11:27:46 PDT 2007
On May 22, 2007, at 10:41 PM, Reid Spencer wrote:
> On Tue, 22 May 2007 22:12:17 -0700
> Chris Lattner <clattner at apple.com> wrote:
>>
>> On May 22, 2007, at 12:33 PM, Reid Spencer wrote:
>>
>>> The attached patch contains a small modification to get the index
>>> of the
>>> iAny types correct for overloaded intrinsics. It has been tested and
>>> passes DejaGnu and MultiSource. Please apply.
>>
>> Can you please explain what bad case this (and the previous patch)
>> fixes? Is this a .ll/.bc syntax change?
>
> No, its not a .ll/.bc syntax change, but it is a slight API
> correction. The issue has to do with the Intrinsic::getDeclaration
> method and the interpretation of the (often defaulted) 3rd and 4th
> parameters that specify the actual types to "fill" in for the iAny
> parameters (or result). The third argument is an array of Type*
> and the fourth is the number of elements. Previously you had to
> specify an entry in the array for every parameter and the result
> type, regardless of whether they were iAny or not. This is
> inconsistent with uses of Intrinsic::getDeclaration when there are
> no iAny parameters in which case it is permissable to pass 0 for
> both the 3rd and 4th arguments (and those happen to the be the
> default values).
Makes sense, thanks for the clarification reid!
-Chris
> For example, cttz and friends take an iAny parameter but return
> i32. Consequently they require only one type in the "Tys" argument
> to getDeclaration (for the parameter). Previously tblgen was
> indexing into "Tys" using the parameter index (or 0 for the result
> type) but this leads to "holes" (zero valued Type*) in the array.
> The interface was changed to eliminate these holes so that the only
> things passed in Tys is the actual types for the iAny arguments.
>
> Consider this, from LangRef.html:
>
> declare i32 @llvm.ctpop.i8 (i8 <src>)
>
> The single suffix (.i8) that qualifies the argument type indicates
> that you should be able to call getDeclaration with:
>
> Type *Tys[] = { Type::Int8Ty };
> Function *F = Intrinsics::getDeclaration(Mod, Intrinsics::ctpop,
> Tys, 1);
>
> However, such a call previously caused an assertion (or worse). The
> problem is that the code generated by tblgen was indexing into Tys
> with a subscript of 1 (to match the first argument) but the array
> was only 1 item long. Consequently, you'd read junk from the stack
> and assert (or worse).
>
> Previously one would have had to pass, to getDeclaration, something
> like:
>
> Type *Tys[] = { 0, Type::Int8Ty };
> Function *F = Intrinsics::getDeclaration(Mod, Intrinsics::ctpop,
> Tys, 2);
>
> which is counter-intuitive since you don't have to use the 3rd and
> 4th parameters at all (defaulting them to 0) if there are no iAny
> arguments. The function changed in the llvm-gcc patch was doing
> exactly this (an array of 2 with a 0 first element) and I changed
> it to use the former example (single element array).
>
> This was noticed by AutoESL when they tried to call getDeclaration
> directly themselves and ended up getting assertions and verifier
> failures. This wasn't noticed before in LLVM because the only use
> of an overloaded intrinsic in LLVM is bswap and that one is a
> degenerate case. Both its argument and its result are iAny so you
> end up needing an array that has two elements anyway - one for the
> result, one for the argument. However, in cases like part_select,
> part_set, cttz, ctlz and ctpop, that is not the case.
>
>>
>> -Chris
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list