From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id D9CF63858C39 for ; Fri, 13 Jan 2023 16:47:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D9CF63858C39 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=foss.arm.com Authentication-Results: sourceware.org; spf=none smtp.mailfrom=foss.arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 8BC40FEC; Fri, 13 Jan 2023 08:48:14 -0800 (PST) Received: from [10.2.78.76] (unknown [10.2.78.76]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id CC5393F67D; Fri, 13 Jan 2023 08:47:31 -0800 (PST) Message-ID: Date: Fri, 13 Jan 2023 16:47:30 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [PATCH] arm: Split up MVE _Generic associations to prevent type clashes [PR107515] To: Stam Markianos-Wright , "gcc-patches@gcc.gnu.org" Cc: Kyrylo Tkachov References: Content-Language: en-GB From: Richard Earnshaw In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3494.0 required=5.0 tests=BAYES_00,BODY_8BITS,GIT_PATCH_0,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,KAM_LOTSOFHASH,KAM_SHORT,NICE_REPLY_A,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 01/12/2022 18:19, Stam Markianos-Wright via Gcc-patches wrote: > Hi all, > > With these previous patches: > https://gcc.gnu.org/pipermail/gcc-patches/2022-November/606586.html > https://gcc.gnu.org/pipermail/gcc-patches/2022-November/606587.html > we enabled the MVE overloaded _Generic associations to handle more > scalar types, however at PR 107515 we found a new regression that > wasn't detected in our testing: > > With glibc's `posix/types.h`: > ``` > typedef signed int __int32_t; > ... > typedef __int32_t int32_t; > ``` > We would get a `error: '_Generic' specifies two compatible types` > from `__ARM_mve_coerce3` because of `type: param`, when `type` is > `int` and `int32_t: param` both being the same under the hood. > > The same did not happen with Newlib's header `sys/_stdint.h`: > ``` > typedef long int __int32_t; > ... > typedef __int32_t int32_t ; > ``` > which worked fine, because it uses `long int`. > > The same could feasibly happen in `__ARM_mve_coerce2` between > `__fp16` and `float16_t`. > > The solution here is to break the _Generic down, so that the similar > types don't appear at the same level, as is done in `__ARM_mve_typeid`. > > Ok for trunk? > > Thanks, > Stam Markianos-Wright > > gcc/ChangeLog: >         PR target/96795 >         PR target/107515 >         * config/arm/arm_mve.h (__ARM_mve_coerce2): Split types. >         (__ARM_mve_coerce3): Likewise. > > gcc/testsuite/ChangeLog: >         PR target/96795 >         PR target/107515 >         * > gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c: New test. >         * > gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c: New test. Please fix the missing new lines at the end of the tests. Otherwise OK. R. > > > =========== Inline Ctrl+C, Ctrl+V or patch =========== > > diff --git a/gcc/config/arm/arm_mve.h b/gcc/config/arm/arm_mve.h > index > 09167ec118ed3310c5077145e119196f29d83cac..70003653db65736fcfd019e83d9f18153be650dc 100644 > --- a/gcc/config/arm/arm_mve.h > +++ b/gcc/config/arm/arm_mve.h > @@ -35659,9 +35659,9 @@ extern void *__ARM_undef; >  #define __ARM_mve_coerce1(param, type) \ >      _Generic(param, type: param, const type: param, default: *(type > *)__ARM_undef) >  #define __ARM_mve_coerce2(param, type) \ > -    _Generic(param, type: param, float16_t: param, float32_t: param, > default: *(type *)__ARM_undef) > +    _Generic(param, type: param, __fp16: param, default: _Generic > (param, _Float16: param, float16_t: param, float32_t: param, default: > *(type *)__ARM_undef)) >  #define __ARM_mve_coerce3(param, type) \ > -    _Generic(param, type: param, int8_t: param, int16_t: param, > int32_t: param, int64_t: param, uint8_t: param, uint16_t: param, > uint32_t: param, uint64_t: param, default: *(type *)__ARM_undef) > +    _Generic(param, type: param, default: _Generic (param, int8_t: > param, int16_t: param, int32_t: param, int64_t: param, uint8_t: param, > uint16_t: param, uint32_t: param, uint64_t: param, default: *(type > *)__ARM_undef)) > >  #if (__ARM_FEATURE_MVE & 2) /* MVE Floating point.  */ > > diff --git > a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c > new file mode 100644 > index > 0000000000000000000000000000000000000000..427dcacb5ff59b53d5eab1f1582ef6460da3f2f3 > --- /dev/null > +++ > b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-fp.c > @@ -0,0 +1,65 @@ > +/* { dg-require-effective-target arm_v8_1m_mve_fp_ok } */ > +/* { dg-add-options arm_v8_1m_mve_fp } */ > +/* { dg-additional-options "-O2 -Wno-pedantic -Wno-long-long" } */ > +#include "arm_mve.h" > + > +float f1; > +double f2; > +float16_t f3; > +float32_t f4; > +__fp16 f5; > +_Float16 f6; > + > +int i1; > +short i2; > +long i3; > +long long i4; > +int8_t i5; > +int16_t i6; > +int32_t i7; > +int64_t i8; > + > +const int ci1; > +const short ci2; > +const long ci3; > +const long long ci4; > +const int8_t ci5; > +const int16_t ci6; > +const int32_t ci7; > +const int64_t ci8; > + > +float16x8_t floatvec; > +int16x8_t intvec; > + > +void test(void) > +{ > +    /* Test a few different supported ways of passing an int value.  The > +    intrinsic vmulq was chosen arbitrarily, but it is representative of > +    all intrinsics that take a non-const scalar value.  */ > +    intvec = vmulq(intvec, 2); > +    intvec = vmulq(intvec, (int32_t) 2); > +    intvec = vmulq(intvec, (short) 2); > +    intvec = vmulq(intvec, i1); > +    intvec = vmulq(intvec, i2); > +    intvec = vmulq(intvec, i3); > +    intvec = vmulq(intvec, i4); > +    intvec = vmulq(intvec, i5); > +    intvec = vmulq(intvec, i6); > +    intvec = vmulq(intvec, i7); > +    intvec = vmulq(intvec, i8); > + > +    /* Test a few different supported ways of passing a float value.  */ > +    floatvec = vmulq(floatvec, 0.5); > +    floatvec = vmulq(floatvec, 0.5f); > +    floatvec = vmulq(floatvec, (__fp16) 0.5); > +    floatvec = vmulq(floatvec, f1); > +    floatvec = vmulq(floatvec, f2); > +    floatvec = vmulq(floatvec, f3); > +    floatvec = vmulq(floatvec, f4); > +    floatvec = vmulq(floatvec, f5); > +    floatvec = vmulq(floatvec, f6); > +    floatvec = vmulq(floatvec, 0.15f16); > +    floatvec = vmulq(floatvec, (_Float16) 0.15); > +} > + > +/* { dg-final { scan-assembler-not "__ARM_undef" } } */ > \ No newline at end of file > diff --git > a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c > new file mode 100644 > index > 0000000000000000000000000000000000000000..d0e3b3eb30f46cb8327e7b976713990721305c9b > --- /dev/null > +++ > b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_intrinsic_type_overloads-int.c > @@ -0,0 +1,45 @@ > +/* { dg-require-effective-target arm_v8_1m_mve_ok } */ > +/* { dg-add-options arm_v8_1m_mve } */ > +/* { dg-additional-options "-O2 -Wno-pedantic -Wno-long-long" } */ > + > +#include "arm_mve.h" > + > +int i1; > +short i2; > +long i3; > +long long i4; > +int8_t i5; > +int16_t i6; > +int32_t i7; > +int64_t i8; > + > +const int ci1; > +const short ci2; > +const long ci3; > +const long long ci4; > +const int8_t ci5; > +const int16_t ci6; > +const int32_t ci7; > +const int64_t ci8; > + > +int16x8_t intvec; > + > +void test(void) > +{ > +    /* Test a few different supported ways of passing an int value.  The > +    intrinsic vmulq was chosen arbitrarily, but it is representative of > +    all intrinsics that take a non-const scalar value.  */ > +    intvec = vmulq(intvec, 2); > +    intvec = vmulq(intvec, (int32_t) 2); > +    intvec = vmulq(intvec, (short) 2); > +    intvec = vmulq(intvec, i1); > +    intvec = vmulq(intvec, i2); > +    intvec = vmulq(intvec, i3); > +    intvec = vmulq(intvec, i4); > +    intvec = vmulq(intvec, i5); > +    intvec = vmulq(intvec, i6); > +    intvec = vmulq(intvec, i7); > +    intvec = vmulq(intvec, i8); > +} > + > +/* { dg-final { scan-assembler-not "__ARM_undef" } } */ > \ No newline at end of file