Re: [PATCH] Intrinsics for ADCX

[Michael Zolotukhin <michael dot v dot zolotukhin at gmail dot com>]

Hi,
I made a new version of the patch, where I tried to take into account
Uros' remarks - is it ok for trunk?

Bootstrap and new tests are passing, testing is in progress.

Changelog entry:
2012-08-03 Michael Zolotukhin <michael.v.zolotukhin@intel.com>

        * common/config/i386/i386-common.c (OPTION_MASK_ISA_ADX_SET): New.
        (OPTION_MASK_ISA_ADX_UNSET): Likewise.
        (ix86_handle_option): Handle madx option.
        * config.gcc (i[34567]86-*-*): Add adxintrin.h.
        (x86_64-*-*): Likewise.
        * config/i386/adxintrin.h: New header.
        * config/i386/driver-i386.c (host_detect_local_cpu): Detect ADCX/ADOX
        support.
        * config/i386/i386-builtin-types.def
        (UCHAR_FTYPE_UCHAR_UINT_UINT_PINT): New function type.
        (UCHAR_FTYPE_UCHAR_ULONGLONG_ULONGLONG_PINT): Likewise.
        * config/i386/i386-c.c: Define __ADX__ if needed.
        * config/i386/i386.c (ix86_target_string): Define -madx option.
        (PTA_ADX): New.
        (ix86_option_override_internal): Handle new option.
        (ix86_valid_target_attribute_inner_p): Add OPT_madx.
        (ix86_builtins): Add IX86_BUILTIN_ADDCARRYX32,
        IX86_BUILTIN_ADDCARRYX64.
        (ix86_init_mmx_sse_builtins): Define corresponding built-ins.
        (ix86_expand_builtin): Handle these built-ins.
        (ix86_expand_args_builtin): Handle new function types.
        * config/i386/i386.h (TARGET_ADX): New.
        * config/i386/i386.md (adcx<mode>3): New define_insn.
        * config/i386/i386.opt (madx): New.
        * config/i386/x86intrin.h: Include adxintrin.h.

testsuite/Changelog entry:
2012-08-03 Michael Zolotukhin <michael.v.zolotukhin@intel.com>

        * gcc.target/i386/adx-addcarryx32-1.c: New.
        * gcc.target/i386/adx-addcarryx32-2.c: New.
        * gcc.target/i386/adx-addcarryx64-1.c: New.
        * gcc.target/i386/adx-addcarryx64-2.c: New.
        * gcc.target/i386/adx-check.h: New.
        * gcc.target/i386/i386.exp (check_effective_target_adx): New.
        * gcc.target/i386/sse-12.c: Add -madx.
        * gcc.target/i386/sse-13.c: Ditto.
        * gcc.target/i386/sse-14.c: Ditto.
        * gcc.target/i386/sse-22.c: Ditto.
        * gcc.target/i386/sse-23.c: Ditto.
        * g++.dg/other/i386-2.C: Ditto.
        * g++.dg/other/i386-3.C: Ditto.


Thanks, Michael

On 1 August 2012 20:37, Kirill Yukhin <kirill.yukhin@gmail.com> wrote:
> Hi Richard,
>
>> Frankly I don't understand the point of these instructions
>> being added to the ISA at all.  I would have understood an
>> add-with-carry that did *not* modify the flags at all, but
>> two separate ones that modify C and O separately is just
>> downright strange.
> If there is only one carry in flight, they all are equivalent although
> ADOX is a little less useful in loops.
> If there are two carries in flight, that’s where the new instructions
> show their benefit, since they allow accumulation without destroying
> each other (see next comment).
> For any number of carries beyond two, you have to start saving
> restoring carry bits and it degenerates to the first case for some of
> them.
>
>> But to the point: I don't understand the point of having
>> this as a builtin.  Is the code generated by this builtin
>> any better than plain C?
> I think this is just like a practice to introduce new intrinsics for new insns.
> I doubt, that we may generate such things automatically:
> c1 = 0;
> c2 = 0;
> c1 = _adcx64( & res[i], src[i], src2[i], c1);
> c1 = _adcx64( & res[i+1], src[i+1], src2[i+1], c1);
> c2 = _adcx64( & res[i], src[i], src2[i], c2);
> c2 = _adcx64( & res[i+1], src[i+1], src2[i+1], c2);
>
>> And if you're going to have the builtin, why is this restricted
>> to adx anyway?  You obviously can produce the same results with
>> the good old fashioned adc instruction as well.
> We have one intrinsic for both ADCX/ADOX. So, we just picked up first
> one to use when exanding the built-in
>
>> Which begs the question of why you've got a separate pattern
>> for the adx anyway.  If the insn is so much better, it ought to
>> be used in the same pattern we use for adc now.
> I believe, we may introduce global variant of ADCX, which may be
> expanded into either of ADC/ADCX/ADOX on x86 and into analogs
> on the other ports.
>
> K

-- 
---
Best regards,
Michael V. Zolotukhin,
Software Engineer
Intel Corporation.

Attachment: bdw-adx-3.gcc.patch
Description: Binary data