* [PATCH] AArch64: aarch64_class_max_nregs mishandles 64-bit structure modes [PR112577]
@ 2024-01-16 11:05 Tejas Belagod
2024-01-24 11:39 ` Richard Sandiford
0 siblings, 1 reply; 3+ messages in thread
From: Tejas Belagod @ 2024-01-16 11:05 UTC (permalink / raw)
To: gcc-patches; +Cc: Tejas Belagod, richard.sandiford
The target hook aarch64_class_max_nregs returns the incorrect result for 64-bit
structure modes like V31DImode or V41DFmode etc. The calculation of the nregs
is based on the size of AdvSIMD vector register for 64-bit modes which ought to
be UNITS_PER_VREG / 2. This patch fixes the register size.
Existing tests like gcc.target/aarch64/advsimd-intrinsics/vld1x3.c cover this change.
Regression tested on aarch64-linux. Bootstrapped on aarch64-linux.
OK for trunk?
gcc/ChangeLog:
PR target/112577
* config/aarch64/aarch64.cc (aarch64_class_max_nregs): Handle 64-bit
vector structure modes correctly.
---
gcc/config/aarch64/aarch64.cc | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index a5a6b52730d..b9f00bdce3b 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -12914,10 +12914,12 @@ aarch64_class_max_nregs (reg_class_t regclass, machine_mode mode)
&& constant_multiple_p (GET_MODE_SIZE (mode),
aarch64_vl_bytes (mode, vec_flags), &nregs))
return nregs;
- return (vec_flags & VEC_ADVSIMD
- ? CEIL (lowest_size, UNITS_PER_VREG)
- : CEIL (lowest_size, UNITS_PER_WORD));
-
+ if (vec_flags == (VEC_ADVSIMD | VEC_STRUCT | VEC_PARTIAL))
+ return GET_MODE_SIZE (mode).to_constant () / 8;
+ else
+ return (vec_flags & VEC_ADVSIMD
+ ? CEIL (lowest_size, UNITS_PER_VREG)
+ : CEIL (lowest_size, UNITS_PER_WORD));
case PR_REGS:
case PR_LO_REGS:
case PR_HI_REGS:
--
2.25.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] AArch64: aarch64_class_max_nregs mishandles 64-bit structure modes [PR112577]
2024-01-16 11:05 [PATCH] AArch64: aarch64_class_max_nregs mishandles 64-bit structure modes [PR112577] Tejas Belagod
@ 2024-01-24 11:39 ` Richard Sandiford
2024-02-06 5:47 ` Tejas Belagod
0 siblings, 1 reply; 3+ messages in thread
From: Richard Sandiford @ 2024-01-24 11:39 UTC (permalink / raw)
To: Tejas Belagod; +Cc: gcc-patches
Tejas Belagod <tejas.belagod@arm.com> writes:
> The target hook aarch64_class_max_nregs returns the incorrect result for 64-bit
> structure modes like V31DImode or V41DFmode etc. The calculation of the nregs
> is based on the size of AdvSIMD vector register for 64-bit modes which ought to
> be UNITS_PER_VREG / 2. This patch fixes the register size.
>
> Existing tests like gcc.target/aarch64/advsimd-intrinsics/vld1x3.c cover this change.
>
> Regression tested on aarch64-linux. Bootstrapped on aarch64-linux.
>
> OK for trunk?
>
> gcc/ChangeLog:
>
> PR target/112577
> * config/aarch64/aarch64.cc (aarch64_class_max_nregs): Handle 64-bit
> vector structure modes correctly.
> ---
> gcc/config/aarch64/aarch64.cc | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index a5a6b52730d..b9f00bdce3b 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -12914,10 +12914,12 @@ aarch64_class_max_nregs (reg_class_t regclass, machine_mode mode)
> && constant_multiple_p (GET_MODE_SIZE (mode),
> aarch64_vl_bytes (mode, vec_flags), &nregs))
> return nregs;
> - return (vec_flags & VEC_ADVSIMD
> - ? CEIL (lowest_size, UNITS_PER_VREG)
> - : CEIL (lowest_size, UNITS_PER_WORD));
> -
> + if (vec_flags == (VEC_ADVSIMD | VEC_STRUCT | VEC_PARTIAL))
> + return GET_MODE_SIZE (mode).to_constant () / 8;
> + else
> + return (vec_flags & VEC_ADVSIMD
> + ? CEIL (lowest_size, UNITS_PER_VREG)
> + : CEIL (lowest_size, UNITS_PER_WORD));
Very minor, sorry, but I think it would be more usual style to add the
new condition as an early-out and so not add an "else", especially since
there's alreaedy an early-out for SVE above:
if (vec_flags == (VEC_ADVSIMD | VEC_STRUCT | VEC_PARTIAL))
return GET_MODE_SIZE (mode).to_constant () / 8;
return (vec_flags & VEC_ADVSIMD
? CEIL (lowest_size, UNITS_PER_VREG)
: CEIL (lowest_size, UNITS_PER_WORD));
I think it's also worth keeping the blank line between this and the
following block of cases.
OK with that change, thanks.
Richard
> case PR_REGS:
> case PR_LO_REGS:
> case PR_HI_REGS:
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] AArch64: aarch64_class_max_nregs mishandles 64-bit structure modes [PR112577]
2024-01-24 11:39 ` Richard Sandiford
@ 2024-02-06 5:47 ` Tejas Belagod
0 siblings, 0 replies; 3+ messages in thread
From: Tejas Belagod @ 2024-02-06 5:47 UTC (permalink / raw)
To: gcc-patches, richard.sandiford
[-- Attachment #1: Type: text/plain, Size: 2452 bytes --]
On 1/24/24 5:09 PM, Richard Sandiford wrote:
> Tejas Belagod <tejas.belagod@arm.com> writes:
>> The target hook aarch64_class_max_nregs returns the incorrect result for 64-bit
>> structure modes like V31DImode or V41DFmode etc. The calculation of the nregs
>> is based on the size of AdvSIMD vector register for 64-bit modes which ought to
>> be UNITS_PER_VREG / 2. This patch fixes the register size.
>>
>> Existing tests like gcc.target/aarch64/advsimd-intrinsics/vld1x3.c cover this change.
>>
>> Regression tested on aarch64-linux. Bootstrapped on aarch64-linux.
>>
>> OK for trunk?
>>
>> gcc/ChangeLog:
>>
>> PR target/112577
>> * config/aarch64/aarch64.cc (aarch64_class_max_nregs): Handle 64-bit
>> vector structure modes correctly.
>> ---
>> gcc/config/aarch64/aarch64.cc | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
>> index a5a6b52730d..b9f00bdce3b 100644
>> --- a/gcc/config/aarch64/aarch64.cc
>> +++ b/gcc/config/aarch64/aarch64.cc
>> @@ -12914,10 +12914,12 @@ aarch64_class_max_nregs (reg_class_t regclass, machine_mode mode)
>> && constant_multiple_p (GET_MODE_SIZE (mode),
>> aarch64_vl_bytes (mode, vec_flags), &nregs))
>> return nregs;
>> - return (vec_flags & VEC_ADVSIMD
>> - ? CEIL (lowest_size, UNITS_PER_VREG)
>> - : CEIL (lowest_size, UNITS_PER_WORD));
>> -
>> + if (vec_flags == (VEC_ADVSIMD | VEC_STRUCT | VEC_PARTIAL))
>> + return GET_MODE_SIZE (mode).to_constant () / 8;
>> + else
>> + return (vec_flags & VEC_ADVSIMD
>> + ? CEIL (lowest_size, UNITS_PER_VREG)
>> + : CEIL (lowest_size, UNITS_PER_WORD));
>
> Very minor, sorry, but I think it would be more usual style to add the
> new condition as an early-out and so not add an "else", especially since
> there's alreaedy an early-out for SVE above:
>
> if (vec_flags == (VEC_ADVSIMD | VEC_STRUCT | VEC_PARTIAL))
> return GET_MODE_SIZE (mode).to_constant () / 8;
> return (vec_flags & VEC_ADVSIMD
> ? CEIL (lowest_size, UNITS_PER_VREG)
> : CEIL (lowest_size, UNITS_PER_WORD));
>
> I think it's also worth keeping the blank line between this and the
> following block of cases.
>
> OK with that change, thanks.
>
> Richard
Thanks for the review, Richard. Re-spin attached. Will apply.
Thanks,
Tejas.
>
>
>> case PR_REGS:
>> case PR_LO_REGS:
>> case PR_HI_REGS:
[-- Attachment #2: dreg.txt --]
[-- Type: text/plain, Size: 698 bytes --]
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index a5a6b52730d6c5013346d128e89915883f1707ae..a7c624f8b7327ae8c1324959c3ab5dfb4e7ebc6c 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -12914,6 +12914,8 @@ aarch64_class_max_nregs (reg_class_t regclass, machine_mode mode)
&& constant_multiple_p (GET_MODE_SIZE (mode),
aarch64_vl_bytes (mode, vec_flags), &nregs))
return nregs;
+ if (vec_flags == (VEC_ADVSIMD | VEC_STRUCT | VEC_PARTIAL))
+ return GET_MODE_SIZE (mode).to_constant () / 8;
return (vec_flags & VEC_ADVSIMD
? CEIL (lowest_size, UNITS_PER_VREG)
: CEIL (lowest_size, UNITS_PER_WORD));
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-02-06 5:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-16 11:05 [PATCH] AArch64: aarch64_class_max_nregs mishandles 64-bit structure modes [PR112577] Tejas Belagod
2024-01-24 11:39 ` Richard Sandiford
2024-02-06 5:47 ` Tejas Belagod
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).