public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Add support for operand-specific alignment requirements
@ 2023-11-22  9:47 juzhe.zhong
  2023-11-22 10:08 ` Richard Sandiford
  0 siblings, 1 reply; 8+ messages in thread
From: juzhe.zhong @ 2023-11-22  9:47 UTC (permalink / raw)
  To: gcc-patches; +Cc: richard.sandiford, vmakarov, kito.cheng

[-- Attachment #1: Type: text/plain, Size: 619 bytes --]

Hi, Richard.

Thanks for supporting register filter in IRA/LRA.
I found it is useful for RVV since we have a set of widen operations that allow source register overlap highpart of dest register group

For example, if vsext.vf2 v0(dest consume reg v0 and reg v1), v1 (source consume v1 only)
I want to support the highpart overlap above. (Currently, we don't any overlap between source and dest in such instructions).

So, I wonder whether we can pass "machine_mode" into register filter. Ok, I think it's too late since stage 1 closes. I wonder we can add it in GCC-15?

Thanks. 



juzhe.zhong@rivai.ai

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/5] Add support for operand-specific alignment requirements
  2023-11-22  9:47 [PATCH 0/5] Add support for operand-specific alignment requirements juzhe.zhong
@ 2023-11-22 10:08 ` Richard Sandiford
  2023-11-22 22:32   ` 钟居哲
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Sandiford @ 2023-11-22 10:08 UTC (permalink / raw)
  To: juzhe.zhong; +Cc: gcc-patches, vmakarov, kito.cheng

"juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai> writes:
> Hi, Richard.
>
> Thanks for supporting register filter in IRA/LRA.
> I found it is useful for RVV since we have a set of widen operations that allow source register overlap highpart of dest register group
>
> For example, if vsext.vf2 v0(dest consume reg v0 and reg v1), v1 (source consume v1 only)
> I want to support the highpart overlap above. (Currently, we don't any overlap between source and dest in such instructions).
>
> So, I wonder whether we can pass "machine_mode" into register filter. Ok, I think it's too late since stage 1 closes. I wonder we can add it in GCC-15?

I think adding a mode would add too much overhead.  The mode would be
the mode of the operand, but with subregs, the mode of the operand can
be different from the mode of the RA allocno.  So it would no longer
be enough for the RA to calculate a bitmask of filters.  It would need
ro remember which modes are used with those filters.

We'd also need to turn the current HARD_REG_SETs into [MAX_MACHINE_MODE]
arrays of HARD_REG_SETs.  (And there are now more than 256 machine modes
for riscv.)

The pattern that uses the constraints should already "know" the mode.
So if possible, I think it would be better to use different constraints
for different modes, using define_mode_attrs.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Re: [PATCH 0/5] Add support for operand-specific alignment requirements
  2023-11-22 10:08 ` Richard Sandiford
@ 2023-11-22 22:32   ` 钟居哲
  2023-11-23 18:18     ` Richard Sandiford
  0 siblings, 1 reply; 8+ messages in thread
From: 钟居哲 @ 2023-11-22 22:32 UTC (permalink / raw)
  To: richard.sandiford; +Cc: gcc-patches, vmakarov, kito.cheng

[-- Attachment #1: Type: text/plain, Size: 2037 bytes --]

Hi, Richard.

Current define_mode_attr can only map an attribute for a mode.
I wonder whether we can map a mode to multiple attributes ?

E.g. (define_mode_attr dest_constraint [(V16QI "&vr")])

But I want it to be:

(define_mode_attr dest_constraint [(V16QI (TARGET_MIN_VLEN <= 128 "vr") (TARGET_MIN_VLEN > 128 "&vr")) ])

It seems that we can't achieve this for now. Would it be possible we exend it in GCC-15 ?


juzhe.zhong@rivai.ai
 
From: Richard Sandiford
Date: 2023-11-22 18:08
To: juzhe.zhong\@rivai.ai
CC: gcc-patches; vmakarov\@redhat.com; kito.cheng
Subject: Re: [PATCH 0/5] Add support for operand-specific alignment requirements
"juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai> writes:
> Hi, Richard.
>
> Thanks for supporting register filter in IRA/LRA.
> I found it is useful for RVV since we have a set of widen operations that allow source register overlap highpart of dest register group
>
> For example, if vsext.vf2 v0(dest consume reg v0 and reg v1), v1 (source consume v1 only)
> I want to support the highpart overlap above. (Currently, we don't any overlap between source and dest in such instructions).
>
> So, I wonder whether we can pass "machine_mode" into register filter. Ok, I think it's too late since stage 1 closes. I wonder we can add it in GCC-15?
 
I think adding a mode would add too much overhead.  The mode would be
the mode of the operand, but with subregs, the mode of the operand can
be different from the mode of the RA allocno.  So it would no longer
be enough for the RA to calculate a bitmask of filters.  It would need
ro remember which modes are used with those filters.
 
We'd also need to turn the current HARD_REG_SETs into [MAX_MACHINE_MODE]
arrays of HARD_REG_SETs.  (And there are now more than 256 machine modes
for riscv.)
 
The pattern that uses the constraints should already "know" the mode.
So if possible, I think it would be better to use different constraints
for different modes, using define_mode_attrs.
 
Thanks,
Richard
 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/5] Add support for operand-specific alignment requirements
  2023-11-22 22:32   ` 钟居哲
@ 2023-11-23 18:18     ` Richard Sandiford
  2023-11-24  6:42       ` juzhe.zhong
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Sandiford @ 2023-11-23 18:18 UTC (permalink / raw)
  To: 钟居哲; +Cc: gcc-patches, vmakarov, kito.cheng

钟居哲 <juzhe.zhong@rivai.ai> writes:
> Hi, Richard.
>
> Current define_mode_attr can only map an attribute for a mode.
> I wonder whether we can map a mode to multiple attributes ?
>
> E.g. (define_mode_attr dest_constraint [(V16QI "&vr")])
>
> But I want it to be:
>
> (define_mode_attr dest_constraint [(V16QI (TARGET_MIN_VLEN <= 128 "vr") (TARGET_MIN_VLEN > 128 "&vr")) ])
>
> It seems that we can't achieve this for now. Would it be possible we exend it in GCC-15 ?

That construct doesn't seem particularly natural to me.  Could you explain
(conceptually) what kind of insn patterns you need?

Thanks,
Richard

>
>
> juzhe.zhong@rivai.ai
>  
> From: Richard Sandiford
> Date: 2023-11-22 18:08
> To: juzhe.zhong\@rivai.ai
> CC: gcc-patches; vmakarov\@redhat.com; kito.cheng
> Subject: Re: [PATCH 0/5] Add support for operand-specific alignment requirements
> "juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai> writes:
>> Hi, Richard.
>>
>> Thanks for supporting register filter in IRA/LRA.
>> I found it is useful for RVV since we have a set of widen operations that allow source register overlap highpart of dest register group
>>
>> For example, if vsext.vf2 v0(dest consume reg v0 and reg v1), v1 (source consume v1 only)
>> I want to support the highpart overlap above. (Currently, we don't any overlap between source and dest in such instructions).
>>
>> So, I wonder whether we can pass "machine_mode" into register filter. Ok, I think it's too late since stage 1 closes. I wonder we can add it in GCC-15?
>  
> I think adding a mode would add too much overhead.  The mode would be
> the mode of the operand, but with subregs, the mode of the operand can
> be different from the mode of the RA allocno.  So it would no longer
> be enough for the RA to calculate a bitmask of filters.  It would need
> ro remember which modes are used with those filters.
>  
> We'd also need to turn the current HARD_REG_SETs into [MAX_MACHINE_MODE]
> arrays of HARD_REG_SETs.  (And there are now more than 256 machine modes
> for riscv.)
>  
> The pattern that uses the constraints should already "know" the mode.
> So if possible, I think it would be better to use different constraints
> for different modes, using define_mode_attrs.
>  
> Thanks,
> Richard
>  

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Re: [PATCH 0/5] Add support for operand-specific alignment requirements
  2023-11-23 18:18     ` Richard Sandiford
@ 2023-11-24  6:42       ` juzhe.zhong
  2023-11-24  9:55         ` Richard Sandiford
  0 siblings, 1 reply; 8+ messages in thread
From: juzhe.zhong @ 2023-11-24  6:42 UTC (permalink / raw)
  To: richard.sandiford; +Cc: gcc-patches, vmakarov, Kito.cheng

[-- Attachment #1: Type: text/plain, Size: 3339 bytes --]

Hi, Richard.

Here is an example for vsext.vf2.

The general pattern for this instruction as follows:

(set (operand 0) (unspec:...(operand 1)))

We have a TARGET_MIN_VLEN macro which specify minimum VLEN according to -march

Consider this case V16QI  -> V16HI of vsext.vf2


When TARGET_MIN_VLEN == 128
The constraint for vsext.vf should be:

 (set (operand 0: V16HI "=vr,&vr") (unspec:...(operand 1: V16QI "register filter1, vr")))

Wheras when TARGET_MIN_VLEN > 128,
We are expecting the constraint:
(set (operand 0: V16HI "=&vr") (unspec:...(operand 1: V16QI "vr")))

That is, same mode, same instruction pattern. We want the constraint to be different according to TARGET_MIN_VLEN.

Currently, I don't know whether we can have an approach to support this feature.

Thanks.


juzhe.zhong@rivai.ai
 
From: Richard Sandiford
Date: 2023-11-24 02:18
To: 钟居哲
CC: gcc-patches; vmakarov; kito.cheng
Subject: Re: [PATCH 0/5] Add support for operand-specific alignment requirements
钟居哲 <juzhe.zhong@rivai.ai> writes:
> Hi, Richard.
>
> Current define_mode_attr can only map an attribute for a mode.
> I wonder whether we can map a mode to multiple attributes ?
>
> E.g. (define_mode_attr dest_constraint [(V16QI "&vr")])
>
> But I want it to be:
>
> (define_mode_attr dest_constraint [(V16QI (TARGET_MIN_VLEN <= 128 "vr") (TARGET_MIN_VLEN > 128 "&vr")) ])
>
> It seems that we can't achieve this for now. Would it be possible we exend it in GCC-15 ?
 
That construct doesn't seem particularly natural to me.  Could you explain
(conceptually) what kind of insn patterns you need?
 
Thanks,
Richard
 
>
>
> juzhe.zhong@rivai.ai
>  
> From: Richard Sandiford
> Date: 2023-11-22 18:08
> To: juzhe.zhong\@rivai.ai
> CC: gcc-patches; vmakarov\@redhat.com; kito.cheng
> Subject: Re: [PATCH 0/5] Add support for operand-specific alignment requirements
> "juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai> writes:
>> Hi, Richard.
>>
>> Thanks for supporting register filter in IRA/LRA.
>> I found it is useful for RVV since we have a set of widen operations that allow source register overlap highpart of dest register group
>>
>> For example, if vsext.vf2 v0(dest consume reg v0 and reg v1), v1 (source consume v1 only)
>> I want to support the highpart overlap above. (Currently, we don't any overlap between source and dest in such instructions).
>>
>> So, I wonder whether we can pass "machine_mode" into register filter. Ok, I think it's too late since stage 1 closes. I wonder we can add it in GCC-15?
>  
> I think adding a mode would add too much overhead.  The mode would be
> the mode of the operand, but with subregs, the mode of the operand can
> be different from the mode of the RA allocno.  So it would no longer
> be enough for the RA to calculate a bitmask of filters.  It would need
> ro remember which modes are used with those filters.
>  
> We'd also need to turn the current HARD_REG_SETs into [MAX_MACHINE_MODE]
> arrays of HARD_REG_SETs.  (And there are now more than 256 machine modes
> for riscv.)
>  
> The pattern that uses the constraints should already "know" the mode.
> So if possible, I think it would be better to use different constraints
> for different modes, using define_mode_attrs.
>  
> Thanks,
> Richard
>  
 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/5] Add support for operand-specific alignment requirements
  2023-11-24  6:42       ` juzhe.zhong
@ 2023-11-24  9:55         ` Richard Sandiford
  0 siblings, 0 replies; 8+ messages in thread
From: Richard Sandiford @ 2023-11-24  9:55 UTC (permalink / raw)
  To: juzhe.zhong; +Cc: gcc-patches, vmakarov, Kito.cheng

"juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai> writes:
> Hi, Richard.
>
> Here is an example for vsext.vf2.
>
> The general pattern for this instruction as follows:
>
> (set (operand 0) (unspec:...(operand 1)))
>
> We have a TARGET_MIN_VLEN macro which specify minimum VLEN according to -march
>
> Consider this case V16QI  -> V16HI of vsext.vf2
>
>
> When TARGET_MIN_VLEN == 128
> The constraint for vsext.vf should be:
>
>  (set (operand 0: V16HI "=vr,&vr") (unspec:...(operand 1: V16QI "register filter1, vr")))
>
> Wheras when TARGET_MIN_VLEN > 128,
> We are expecting the constraint:
> (set (operand 0: V16HI "=&vr") (unspec:...(operand 1: V16QI "vr")))
>
> That is, same mode, same instruction pattern. We want the constraint to be different according to TARGET_MIN_VLEN.
>
> Currently, I don't know whether we can have an approach to support this feature.

It looks like TARGET_MIN_VLEN == 128 provides an extra alternative on
top of what TARGET_MIN_VLEN > 128 provides.  Is that right?  If so, the
usual way to do that is to provide both alternatives, and use the "enabled"
attribute to restrict the first alternative to TARGET_MIN_VLEN == 128.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/5] Add support for operand-specific alignment requirements
  2023-11-12 14:52 Richard Sandiford
@ 2023-11-14  0:01 ` Vladimir Makarov
  0 siblings, 0 replies; 8+ messages in thread
From: Vladimir Makarov @ 2023-11-14  0:01 UTC (permalink / raw)
  To: Richard Sandiford, jlaw, gcc-patches


On 11/12/23 09:52, Richard Sandiford wrote:
> SME has various instructions that require aligned register tuples.
> However, the associated tuple modes are already widely used and do
> not need to be aligned in other contexts.  It therefore isn't
> appropriate to force alignment in TARGET_HARD_REGNO_MODE_OK.
>
> There are also strided loads and stores that require:
>
> - (regno & 0x8) == 0 for 2-register tuples
> - (regno & 0xc) == 0 for 4-register tuples
>
> Although the requirements for strided loads and stores could be
> enforced by C++ conditions on the insn, it's convenient to handle
> them in the same way as alignment.
>
> This series of patches therefore adds a way for register constraints
> to specify which start registers are valid and which aren't.  Most of
> the details are in the covering note to the first patch.
>
> This is clearly changing a performance-sensitive part of the compiler.
> I've tried to ensure that the overhead is only small for targets that
> use the new feature.  Almost all of the new code gets optimised away
> on targets that don't use the feature.
>
> Richard Sandiford (5):
>    Add register filter operand to define_register_constraint
>    recog: Handle register filters
>    lra: Handle register filters
>    ira: Handle register filters
>    Add an aligned_register_operand predicate
>
>   gcc/common.md          |  28 ++++++++
>   gcc/doc/md.texi        |  41 +++++++++++-
>   gcc/doc/tm.texi        |   3 +-
>   gcc/doc/tm.texi.in     |   3 +-
>   gcc/genconfig.cc       |   2 +
>   gcc/genpreds.cc        | 146 ++++++++++++++++++++++++++++++++++++++++-
>   gcc/gensupport.cc      |  48 +++++++++++++-
>   gcc/gensupport.h       |   3 +
>   gcc/ira-build.cc       |   8 +++
>   gcc/ira-color.cc       |  10 +++
>   gcc/ira-int.h          |  14 ++++
>   gcc/ira-lives.cc       |  61 +++++++++++++++++
>   gcc/lra-constraints.cc |  13 +++-
>   gcc/recog.cc           |  14 +++-
>   gcc/recog.h            |  24 ++++++-
>   gcc/reginfo.cc         |   5 ++
>   gcc/rtl.def            |   6 +-
>   gcc/target-globals.cc  |   6 +-
>   gcc/target-globals.h   |   3 +
>   19 files changed, 421 insertions(+), 17 deletions(-)
>
Collecting all occurrence constraints for IRA probably might result in 
worse allocation (when pseudo is spilled because of this) in comparison 
with using wider hard reg set and generating reload insns for some 
pseudo occurrences requiring stricter constraints.  Regional RA 
mitigates this issue.  In any case IRA changes is an improvement in 
comparison with using only hard_regno_mode_ok.  Using smaller 
constraints in certain cases for pseudos spilled after using the biggest 
constraint is just an idea for further RA improvement for targets using 
the filters. The only question is it worth to implement.

All IRA/LRA/reginfo patches are OK for me.  IMHO other changes are 
pretty strait forward not to ask somebody to review them.

Thank you, Richard.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 0/5] Add support for operand-specific alignment requirements
@ 2023-11-12 14:52 Richard Sandiford
  2023-11-14  0:01 ` Vladimir Makarov
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Sandiford @ 2023-11-12 14:52 UTC (permalink / raw)
  To: jlaw, vmakarov, gcc-patches; +Cc: Richard Sandiford

SME has various instructions that require aligned register tuples.
However, the associated tuple modes are already widely used and do
not need to be aligned in other contexts.  It therefore isn't
appropriate to force alignment in TARGET_HARD_REGNO_MODE_OK.

There are also strided loads and stores that require:

- (regno & 0x8) == 0 for 2-register tuples
- (regno & 0xc) == 0 for 4-register tuples

Although the requirements for strided loads and stores could be
enforced by C++ conditions on the insn, it's convenient to handle
them in the same way as alignment.

This series of patches therefore adds a way for register constraints
to specify which start registers are valid and which aren't.  Most of
the details are in the covering note to the first patch.

This is clearly changing a performance-sensitive part of the compiler.
I've tried to ensure that the overhead is only small for targets that
use the new feature.  Almost all of the new code gets optimised away
on targets that don't use the feature.

Richard Sandiford (5):
  Add register filter operand to define_register_constraint
  recog: Handle register filters
  lra: Handle register filters
  ira: Handle register filters
  Add an aligned_register_operand predicate

 gcc/common.md          |  28 ++++++++
 gcc/doc/md.texi        |  41 +++++++++++-
 gcc/doc/tm.texi        |   3 +-
 gcc/doc/tm.texi.in     |   3 +-
 gcc/genconfig.cc       |   2 +
 gcc/genpreds.cc        | 146 ++++++++++++++++++++++++++++++++++++++++-
 gcc/gensupport.cc      |  48 +++++++++++++-
 gcc/gensupport.h       |   3 +
 gcc/ira-build.cc       |   8 +++
 gcc/ira-color.cc       |  10 +++
 gcc/ira-int.h          |  14 ++++
 gcc/ira-lives.cc       |  61 +++++++++++++++++
 gcc/lra-constraints.cc |  13 +++-
 gcc/recog.cc           |  14 +++-
 gcc/recog.h            |  24 ++++++-
 gcc/reginfo.cc         |   5 ++
 gcc/rtl.def            |   6 +-
 gcc/target-globals.cc  |   6 +-
 gcc/target-globals.h   |   3 +
 19 files changed, 421 insertions(+), 17 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-11-24  9:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-22  9:47 [PATCH 0/5] Add support for operand-specific alignment requirements juzhe.zhong
2023-11-22 10:08 ` Richard Sandiford
2023-11-22 22:32   ` 钟居哲
2023-11-23 18:18     ` Richard Sandiford
2023-11-24  6:42       ` juzhe.zhong
2023-11-24  9:55         ` Richard Sandiford
  -- strict thread matches above, loose matches on Subject: below --
2023-11-12 14:52 Richard Sandiford
2023-11-14  0:01 ` Vladimir Makarov

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).