diff mbox series

[v3,2/8] aarch64: Make C/C++ operations possible on SVE ACLE types.

Message ID 20241129035404.3363162-3-tejas.belagod@arm.com
State Committed
Commit 761cf60218890af31cfb4a4cebc4509d3262a9bb
Headers
Series aarch64: Enable C/C++ operations on SVE ACLE types. |

Commit Message

Tejas Belagod Nov. 29, 2024, 3:53 a.m. UTC 
  This patch changes the TYPE_INDIVISBLE flag to 0 to enable SVE ACLE types to be
treated as GNU vectors and have the same semantics with operations that are
defined on GNU vectors.

gcc/ChangeLog:

	* config/aarch64/aarch64-sve-builtins.cc (register_builtin_types): Flip
	TYPE_INDIVISBLE flag for SVE ACLE vector types.
---
 gcc/config/aarch64/aarch64-sve-builtins.cc | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Christophe Lyon Nov. 29, 2024, 10 p.m. UTC | #1 
Hi!

On Fri, 29 Nov 2024 at 05:00, Tejas Belagod <tejas.belagod@arm.com> wrote:
>
> This patch changes the TYPE_INDIVISBLE flag to 0 to enable SVE ACLE types to be
> treated as GNU vectors and have the same semantics with operations that are
> defined on GNU vectors.
>
> gcc/ChangeLog:
>
>         * config/aarch64/aarch64-sve-builtins.cc (register_builtin_types): Flip
>         TYPE_INDIVISBLE flag for SVE ACLE vector types.

Sorry I haven't closely followed the discussions around this patch
series, but the Linaro postcommit CI reports
1036 regressions after patch 2/8, is that expected?
Given that precommit CI detected "only" 22 regressions with all 8
patches, I suppose most of the 1036 are fixed later in the series?

Thanks,

Christophe

> ---
>  gcc/config/aarch64/aarch64-sve-builtins.cc | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc
> index 0fec1cd439e..adbadd303d4 100644
> --- a/gcc/config/aarch64/aarch64-sve-builtins.cc
> +++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
> @@ -4576,6 +4576,9 @@ register_builtin_types ()
>               vectype = build_truth_vector_type_for_mode (BYTES_PER_SVE_VECTOR,
>                                                           VNx16BImode);
>               num_pr = 1;
> +             /* Leave svbool_t as indivisible for now.  We don't yet support
> +                C/C++ operators on predicates.  */
> +             TYPE_INDIVISIBLE_P (vectype) = 1;
>             }
>           else
>             {
> @@ -4592,12 +4595,12 @@ register_builtin_types ()
>                           && TYPE_ALIGN (vectype) == 128
>                           && known_eq (size, BITS_PER_SVE_VECTOR));
>               num_zr = 1;
> +             TYPE_INDIVISIBLE_P (vectype) = 0;
>             }
>           vectype = build_distinct_type_copy (vectype);
>           gcc_assert (vectype == TYPE_MAIN_VARIANT (vectype));
>           SET_TYPE_STRUCTURAL_EQUALITY (vectype);
>           TYPE_ARTIFICIAL (vectype) = 1;
> -         TYPE_INDIVISIBLE_P (vectype) = 1;
>           make_type_sizeless (vectype);
>         }
>        if (num_pr)
> --
> 2.25.1
>
Tejas Belagod Dec. 2, 2024, 9:45 a.m. UTC | #2 
On 11/30/24 3:30 AM, Christophe Lyon wrote:
> Hi!
> 
> On Fri, 29 Nov 2024 at 05:00, Tejas Belagod <tejas.belagod@arm.com> wrote:
>>
>> This patch changes the TYPE_INDIVISBLE flag to 0 to enable SVE ACLE types to be
>> treated as GNU vectors and have the same semantics with operations that are
>> defined on GNU vectors.
>>
>> gcc/ChangeLog:
>>
>>          * config/aarch64/aarch64-sve-builtins.cc (register_builtin_types): Flip
>>          TYPE_INDIVISBLE flag for SVE ACLE vector types.
> 
> Sorry I haven't closely followed the discussions around this patch
> series, but the Linaro postcommit CI reports
> 1036 regressions after patch 2/8, is that expected?
> Given that precommit CI detected "only" 22 regressions with all 8
> patches, I suppose most of the 1036 are fixed later in the series?

Thanks for raising this.

Patch 2/8 enables SVE vectors to behave like GNU vectors (C/C++ operator 
semantics start applying to SVE vectors) which has a lot of fallout in 
FE/ME/BE that the patches 3-8 fix (mostly related to handling VLA vectors).

I'm currently testing a patch to fix the remaining 22 regressions - they 
are mostly testisms for which I wanted to make sure I was doing the 
right thing (which I have indicated in my cover letter).

Thanks,
Tejas.

> 
> Thanks,
> 
> Christophe
> 
>> ---
>>   gcc/config/aarch64/aarch64-sve-builtins.cc | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc
>> index 0fec1cd439e..adbadd303d4 100644
>> --- a/gcc/config/aarch64/aarch64-sve-builtins.cc
>> +++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
>> @@ -4576,6 +4576,9 @@ register_builtin_types ()
>>                vectype = build_truth_vector_type_for_mode (BYTES_PER_SVE_VECTOR,
>>                                                            VNx16BImode);
>>                num_pr = 1;
>> +             /* Leave svbool_t as indivisible for now.  We don't yet support
>> +                C/C++ operators on predicates.  */
>> +             TYPE_INDIVISIBLE_P (vectype) = 1;
>>              }
>>            else
>>              {
>> @@ -4592,12 +4595,12 @@ register_builtin_types ()
>>                            && TYPE_ALIGN (vectype) == 128
>>                            && known_eq (size, BITS_PER_SVE_VECTOR));
>>                num_zr = 1;
>> +             TYPE_INDIVISIBLE_P (vectype) = 0;
>>              }
>>            vectype = build_distinct_type_copy (vectype);
>>            gcc_assert (vectype == TYPE_MAIN_VARIANT (vectype));
>>            SET_TYPE_STRUCTURAL_EQUALITY (vectype);
>>            TYPE_ARTIFICIAL (vectype) = 1;
>> -         TYPE_INDIVISIBLE_P (vectype) = 1;
>>            make_type_sizeless (vectype);
>>          }
>>         if (num_pr)
>> --
>> 2.25.1
>>
Andrew Pinski Dec. 2, 2024, 9:50 a.m. UTC | #3 
On Mon, Dec 2, 2024 at 1:47 AM Tejas Belagod <tejas.belagod@arm.com> wrote:
>
> On 11/30/24 3:30 AM, Christophe Lyon wrote:
> > Hi!
> >
> > On Fri, 29 Nov 2024 at 05:00, Tejas Belagod <tejas.belagod@arm.com> wrote:
> >>
> >> This patch changes the TYPE_INDIVISBLE flag to 0 to enable SVE ACLE types to be
> >> treated as GNU vectors and have the same semantics with operations that are
> >> defined on GNU vectors.
> >>
> >> gcc/ChangeLog:
> >>
> >>          * config/aarch64/aarch64-sve-builtins.cc (register_builtin_types): Flip
> >>          TYPE_INDIVISBLE flag for SVE ACLE vector types.
> >
> > Sorry I haven't closely followed the discussions around this patch
> > series, but the Linaro postcommit CI reports
> > 1036 regressions after patch 2/8, is that expected?
> > Given that precommit CI detected "only" 22 regressions with all 8
> > patches, I suppose most of the 1036 are fixed later in the series?
>
> Thanks for raising this.
>
> Patch 2/8 enables SVE vectors to behave like GNU vectors (C/C++ operator
> semantics start applying to SVE vectors) which has a lot of fallout in
> FE/ME/BE that the patches 3-8 fix (mostly related to handling VLA vectors).
>
> I'm currently testing a patch to fix the remaining 22 regressions - they
> are mostly testisms for which I wanted to make sure I was doing the
> right thing (which I have indicated in my cover letter).

I think I fixed all of the testcases over the weekend.

https://gcc.gnu.org/pipermail/gcc-patches/2024-December/670492.html
https://gcc.gnu.org/pipermail/gcc-patches/2024-December/670493.html
https://gcc.gnu.org/pipermail/gcc-patches/2024-December/670494.html
https://gcc.gnu.org/pipermail/gcc-patches/2024-December/670495.html
https://gcc.gnu.org/pipermail/gcc-patches/2024-December/670496.html
https://gcc.gnu.org/pipermail/gcc-patches/2024-December/670497.html

Thanks,
Andrew

>
> Thanks,
> Tejas.
>
> >
> > Thanks,
> >
> > Christophe
> >
> >> ---
> >>   gcc/config/aarch64/aarch64-sve-builtins.cc | 5 ++++-
> >>   1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc
> >> index 0fec1cd439e..adbadd303d4 100644
> >> --- a/gcc/config/aarch64/aarch64-sve-builtins.cc
> >> +++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
> >> @@ -4576,6 +4576,9 @@ register_builtin_types ()
> >>                vectype = build_truth_vector_type_for_mode (BYTES_PER_SVE_VECTOR,
> >>                                                            VNx16BImode);
> >>                num_pr = 1;
> >> +             /* Leave svbool_t as indivisible for now.  We don't yet support
> >> +                C/C++ operators on predicates.  */
> >> +             TYPE_INDIVISIBLE_P (vectype) = 1;
> >>              }
> >>            else
> >>              {
> >> @@ -4592,12 +4595,12 @@ register_builtin_types ()
> >>                            && TYPE_ALIGN (vectype) == 128
> >>                            && known_eq (size, BITS_PER_SVE_VECTOR));
> >>                num_zr = 1;
> >> +             TYPE_INDIVISIBLE_P (vectype) = 0;
> >>              }
> >>            vectype = build_distinct_type_copy (vectype);
> >>            gcc_assert (vectype == TYPE_MAIN_VARIANT (vectype));
> >>            SET_TYPE_STRUCTURAL_EQUALITY (vectype);
> >>            TYPE_ARTIFICIAL (vectype) = 1;
> >> -         TYPE_INDIVISIBLE_P (vectype) = 1;
> >>            make_type_sizeless (vectype);
> >>          }
> >>         if (num_pr)
> >> --
> >> 2.25.1
> >>
>
Christophe Lyon Dec. 2, 2024, 9:54 a.m. UTC | #4 
On Mon, 2 Dec 2024 at 10:45, Tejas Belagod <tejas.belagod@arm.com> wrote:
>
> On 11/30/24 3:30 AM, Christophe Lyon wrote:
> > Hi!
> >
> > On Fri, 29 Nov 2024 at 05:00, Tejas Belagod <tejas.belagod@arm.com> wrote:
> >>
> >> This patch changes the TYPE_INDIVISBLE flag to 0 to enable SVE ACLE types to be
> >> treated as GNU vectors and have the same semantics with operations that are
> >> defined on GNU vectors.
> >>
> >> gcc/ChangeLog:
> >>
> >>          * config/aarch64/aarch64-sve-builtins.cc (register_builtin_types): Flip
> >>          TYPE_INDIVISBLE flag for SVE ACLE vector types.
> >
> > Sorry I haven't closely followed the discussions around this patch
> > series, but the Linaro postcommit CI reports
> > 1036 regressions after patch 2/8, is that expected?
> > Given that precommit CI detected "only" 22 regressions with all 8
> > patches, I suppose most of the 1036 are fixed later in the series?
>
> Thanks for raising this.
>
> Patch 2/8 enables SVE vectors to behave like GNU vectors (C/C++ operator
> semantics start applying to SVE vectors) which has a lot of fallout in
> FE/ME/BE that the patches 3-8 fix (mostly related to handling VLA vectors).
>
> I'm currently testing a patch to fix the remaining 22 regressions - they
> are mostly testisms for which I wanted to make sure I was doing the
> right thing (which I have indicated in my cover letter).
>
Indeed, but I wasn't expecting regressions within the series, also
fixed within the series (I thought the policy was to avoid such
things, and that each patch is expected not to introduce regressions,
precisely because it's annoying during bisects).
As you have probably noticed the CI has sent other notifications with
your follow-up patches, finding a few more regressions and indeed lots
of improvements.  I'll have a quick check and probably close them as
these results look to be expected from your side.

Regarding the 22 regressions you mention above, I've noticed that
Andrew Pinksi has fixed some (all?) of them already (in case you
haven't noticed his patches).

Thanks,

Christophe


> Thanks,
> Tejas.
>
> >
> > Thanks,
> >
> > Christophe
> >
> >> ---
> >>   gcc/config/aarch64/aarch64-sve-builtins.cc | 5 ++++-
> >>   1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc
> >> index 0fec1cd439e..adbadd303d4 100644
> >> --- a/gcc/config/aarch64/aarch64-sve-builtins.cc
> >> +++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
> >> @@ -4576,6 +4576,9 @@ register_builtin_types ()
> >>                vectype = build_truth_vector_type_for_mode (BYTES_PER_SVE_VECTOR,
> >>                                                            VNx16BImode);
> >>                num_pr = 1;
> >> +             /* Leave svbool_t as indivisible for now.  We don't yet support
> >> +                C/C++ operators on predicates.  */
> >> +             TYPE_INDIVISIBLE_P (vectype) = 1;
> >>              }
> >>            else
> >>              {
> >> @@ -4592,12 +4595,12 @@ register_builtin_types ()
> >>                            && TYPE_ALIGN (vectype) == 128
> >>                            && known_eq (size, BITS_PER_SVE_VECTOR));
> >>                num_zr = 1;
> >> +             TYPE_INDIVISIBLE_P (vectype) = 0;
> >>              }
> >>            vectype = build_distinct_type_copy (vectype);
> >>            gcc_assert (vectype == TYPE_MAIN_VARIANT (vectype));
> >>            SET_TYPE_STRUCTURAL_EQUALITY (vectype);
> >>            TYPE_ARTIFICIAL (vectype) = 1;
> >> -         TYPE_INDIVISIBLE_P (vectype) = 1;
> >>            make_type_sizeless (vectype);
> >>          }
> >>         if (num_pr)
> >> --
> >> 2.25.1
> >>
>
Tejas Belagod Dec. 2, 2024, 9:57 a.m. UTC | #5 
On 12/2/24 3:20 PM, Andrew Pinski wrote:
> On Mon, Dec 2, 2024 at 1:47 AM Tejas Belagod <tejas.belagod@arm.com> wrote:
>>
>> On 11/30/24 3:30 AM, Christophe Lyon wrote:
>>> Hi!
>>>
>>> On Fri, 29 Nov 2024 at 05:00, Tejas Belagod <tejas.belagod@arm.com> wrote:
>>>>
>>>> This patch changes the TYPE_INDIVISBLE flag to 0 to enable SVE ACLE types to be
>>>> treated as GNU vectors and have the same semantics with operations that are
>>>> defined on GNU vectors.
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>>           * config/aarch64/aarch64-sve-builtins.cc (register_builtin_types): Flip
>>>>           TYPE_INDIVISBLE flag for SVE ACLE vector types.
>>>
>>> Sorry I haven't closely followed the discussions around this patch
>>> series, but the Linaro postcommit CI reports
>>> 1036 regressions after patch 2/8, is that expected?
>>> Given that precommit CI detected "only" 22 regressions with all 8
>>> patches, I suppose most of the 1036 are fixed later in the series?
>>
>> Thanks for raising this.
>>
>> Patch 2/8 enables SVE vectors to behave like GNU vectors (C/C++ operator
>> semantics start applying to SVE vectors) which has a lot of fallout in
>> FE/ME/BE that the patches 3-8 fix (mostly related to handling VLA vectors).
>>
>> I'm currently testing a patch to fix the remaining 22 regressions - they
>> are mostly testisms for which I wanted to make sure I was doing the
>> right thing (which I have indicated in my cover letter).
> 
> I think I fixed all of the testcases over the weekend.
> 
> https://gcc.gnu.org/pipermail/gcc-patches/2024-December/670492.html
> https://gcc.gnu.org/pipermail/gcc-patches/2024-December/670493.html
> https://gcc.gnu.org/pipermail/gcc-patches/2024-December/670494.html
> https://gcc.gnu.org/pipermail/gcc-patches/2024-December/670495.html
> https://gcc.gnu.org/pipermail/gcc-patches/2024-December/670496.html
> https://gcc.gnu.org/pipermail/gcc-patches/2024-December/670497.html
> 

Ah, sorry, missed that - was still catching up with stuff. Thanks for 
the fixes - much appreciated.

Thanks,
Tejas.

> Thanks,
> Andrew
> 
>>
>> Thanks,
>> Tejas.
>>
>>>
>>> Thanks,
>>>
>>> Christophe
>>>
>>>> ---
>>>>    gcc/config/aarch64/aarch64-sve-builtins.cc | 5 ++++-
>>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc
>>>> index 0fec1cd439e..adbadd303d4 100644
>>>> --- a/gcc/config/aarch64/aarch64-sve-builtins.cc
>>>> +++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
>>>> @@ -4576,6 +4576,9 @@ register_builtin_types ()
>>>>                 vectype = build_truth_vector_type_for_mode (BYTES_PER_SVE_VECTOR,
>>>>                                                             VNx16BImode);
>>>>                 num_pr = 1;
>>>> +             /* Leave svbool_t as indivisible for now.  We don't yet support
>>>> +                C/C++ operators on predicates.  */
>>>> +             TYPE_INDIVISIBLE_P (vectype) = 1;
>>>>               }
>>>>             else
>>>>               {
>>>> @@ -4592,12 +4595,12 @@ register_builtin_types ()
>>>>                             && TYPE_ALIGN (vectype) == 128
>>>>                             && known_eq (size, BITS_PER_SVE_VECTOR));
>>>>                 num_zr = 1;
>>>> +             TYPE_INDIVISIBLE_P (vectype) = 0;
>>>>               }
>>>>             vectype = build_distinct_type_copy (vectype);
>>>>             gcc_assert (vectype == TYPE_MAIN_VARIANT (vectype));
>>>>             SET_TYPE_STRUCTURAL_EQUALITY (vectype);
>>>>             TYPE_ARTIFICIAL (vectype) = 1;
>>>> -         TYPE_INDIVISIBLE_P (vectype) = 1;
>>>>             make_type_sizeless (vectype);
>>>>           }
>>>>          if (num_pr)
>>>> --
>>>> 2.25.1
>>>>
>>
diff mbox series

Patch

diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc
index 0fec1cd439e..adbadd303d4 100644
--- a/gcc/config/aarch64/aarch64-sve-builtins.cc
+++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
@@ -4576,6 +4576,9 @@  register_builtin_types ()
 	      vectype = build_truth_vector_type_for_mode (BYTES_PER_SVE_VECTOR,
 							  VNx16BImode);
 	      num_pr = 1;
+	      /* Leave svbool_t as indivisible for now.  We don't yet support
+		 C/C++ operators on predicates.  */
+	      TYPE_INDIVISIBLE_P (vectype) = 1;
 	    }
 	  else
 	    {
@@ -4592,12 +4595,12 @@  register_builtin_types ()
 			  && TYPE_ALIGN (vectype) == 128
 			  && known_eq (size, BITS_PER_SVE_VECTOR));
 	      num_zr = 1;
+	      TYPE_INDIVISIBLE_P (vectype) = 0;
 	    }
 	  vectype = build_distinct_type_copy (vectype);
 	  gcc_assert (vectype == TYPE_MAIN_VARIANT (vectype));
 	  SET_TYPE_STRUCTURAL_EQUALITY (vectype);
 	  TYPE_ARTIFICIAL (vectype) = 1;
-	  TYPE_INDIVISIBLE_P (vectype) = 1;
 	  make_type_sizeless (vectype);
 	}
       if (num_pr)