Re: PATCH: Add x86 integer intrinsics

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Thu, Jun 11, 2009 at 8:48 PM, Richard Henderson<rth@redhat.com> wrote:
>> +(define_expand "bsr"
>> + ?[(parallel
>> + ? ?[(set (match_operand:SI 0 "register_operand" "")
>> + ? ? ? ? (minus:SI (const_int 31)
>> + ? ? ? ? ? ? ? ? ? (clz:SI (match_operand:SI 1 "nonimmediate_operand"
>> ""))))
>> + ? ? (clobber (reg:CC FLAGS_REG))])]
>> + ?""
>> + ?"")
>> +
>> ?(define_insn "*bsr"
>
> You don't need the expand, just remove the * from the define_insn.

Done.

>> +(define_expand "rdpmc"
>> + ?[(parallel [(set (match_operand:DI 0 "register_operand" "")
>> + ? ? ? ? ? ? ? ? ?(unspec_volatile:DI [(const_int 0)] UNSPECV_RDPMC))
>> + ? ? ? ? ? ? (unspec_volatile [(match_operand:SI 1 "nonimmediate_operand"
>> "")] UNSPECV_RDPMC)])]
>> + ?""
>
> If you're going to give any kind of constraint to op1,
> you might as well have it be register_operand. ?Either
> your builtin expander is going to honor it, or it won't.
>
> And really, there's no point being too complicated here
> with the expand. ?Just use
>
> ?[(match_operand:DI 0 "register_operand" "")
> ?(match_operand:SI 1 "register_operand" "")]
>
>> +(define_insn "*rdpmc"
>> + ?[(parallel [(set (match_operand:DI 0 "register_operand" "=A")
>> + ? ? ? ? ? ? ? ? ?(unspec_volatile:DI [(const_int 0)] UNSPECV_RDPMC))
>> + ? ? ? ? ? ? (unspec_volatile [(match_operand:SI 1 "register_operand"
>> "c")] UNSPECV_RDPMC)])]
>
> define_insn doesn't use an explicit parallel.
> Also, no point in a parallel at all:
>
> ?[(set (match_operand:DI 0 "register_operand" "=A")
> ? ? ? (unspec_volatile:DI [(match_operand:SI 1 "register_operand" "c")]
> ? ? ? ? ? ? ? ? ? ? ? ? ? UNSPECV_RDPMC))]
>
> For 64-bit:
>
> ?[(set (match_operand:DI 0 "register_operand" "=a")
> ? ? ? (unspec_volatile:DI [(match_operand:SI 2 "register_operand" "c")]
> ? ? ? ? ? ? ? ? ? ? ? ? ? UNSPECV_RDPMC))
> ?(set (match_operand:DI 1 "register_operand" "=d")
> ? ? ? (unspec_volatile:DI [(const_int 0)] UNSPEC_RDPMC))]

Done.

> And why aren't you using x86_64_load_64bit_split for all of these?
>

I folded  x86_64_load_64bit_split into rdtsc. 3 patterns are very
different. There is no need for a function just for one pattern.

Here is the updated patch. OK for trunk?

Thanks.


-- 
H.J.

Attachment: gcc-intrin-3.patch
Description: Text document