* [PATCH] Check hard_regno_mode_ok before setting lowest memory move cost for the mode with different reg classes.
@ 2023-04-04 5:13 liuhongt
2023-04-05 1:29 ` Jeff Law
0 siblings, 1 reply; 5+ messages in thread
From: liuhongt @ 2023-04-04 5:13 UTC (permalink / raw)
To: gcc-patches; +Cc: vmakarov
There's a potential performance issue when backend returns some
unreasonable value for the mode which can be never be allocate with
reg class.
Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
Ok for trunk(or GCC14 stage1)?
gcc/ChangeLog:
PR rtl-optimization/109351
* ira.cc (setup_class_subset_and_memory_move_costs): Check
hard_regno_mode_ok before setting lowest memory move cost for
the mode with different reg classes.
---
gcc/ira.cc | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/gcc/ira.cc b/gcc/ira.cc
index 6c7f4901e4c..02dea5d49ee 100644
--- a/gcc/ira.cc
+++ b/gcc/ira.cc
@@ -588,6 +588,10 @@ setup_class_subset_and_memory_move_costs (void)
/* Costs for NO_REGS are used in cost calculation on the
1st pass when the preferred register classes are not
known yet. In this case we take the best scenario. */
+ if (!targetm.hard_regno_mode_ok (ira_class_hard_regs[cl][0],
+ (machine_mode) mode))
+ continue;
+
if (ira_memory_move_cost[mode][NO_REGS][0]
> ira_memory_move_cost[mode][cl][0])
ira_max_memory_move_cost[mode][NO_REGS][0]
--
2.39.1.388.g2fc9e9ca3c
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Check hard_regno_mode_ok before setting lowest memory move cost for the mode with different reg classes.
2023-04-04 5:13 [PATCH] Check hard_regno_mode_ok before setting lowest memory move cost for the mode with different reg classes liuhongt
@ 2023-04-05 1:29 ` Jeff Law
2023-04-05 12:58 ` Vladimir Makarov
0 siblings, 1 reply; 5+ messages in thread
From: Jeff Law @ 2023-04-05 1:29 UTC (permalink / raw)
To: liuhongt, gcc-patches; +Cc: vmakarov
On 4/3/23 23:13, liuhongt via Gcc-patches wrote:
> There's a potential performance issue when backend returns some
> unreasonable value for the mode which can be never be allocate with
> reg class.
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
> Ok for trunk(or GCC14 stage1)?
>
> gcc/ChangeLog:
>
> PR rtl-optimization/109351
> * ira.cc (setup_class_subset_and_memory_move_costs): Check
> hard_regno_mode_ok before setting lowest memory move cost for
> the mode with different reg classes.
Not a regression *and* changing register allocation. This seems like it
should defer to gcc-14.
jeff
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Check hard_regno_mode_ok before setting lowest memory move cost for the mode with different reg classes.
2023-04-05 1:29 ` Jeff Law
@ 2023-04-05 12:58 ` Vladimir Makarov
2023-04-06 5:07 ` Liu, Hongtao
0 siblings, 1 reply; 5+ messages in thread
From: Vladimir Makarov @ 2023-04-05 12:58 UTC (permalink / raw)
To: Jeff Law, liuhongt, gcc-patches
On 4/4/23 21:29, Jeff Law wrote:
>
>
> On 4/3/23 23:13, liuhongt via Gcc-patches wrote:
>> There's a potential performance issue when backend returns some
>> unreasonable value for the mode which can be never be allocate with
>> reg class.
>>
>> Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
>> Ok for trunk(or GCC14 stage1)?
>>
>> gcc/ChangeLog:
>>
>> PR rtl-optimization/109351
>> * ira.cc (setup_class_subset_and_memory_move_costs): Check
>> hard_regno_mode_ok before setting lowest memory move cost for
>> the mode with different reg classes.
> Not a regression *and* changing register allocation. This seems like
> it should defer to gcc-14.
>
Yes, I am agree. It should wait for gcc-14, especially when we are
close to the release. Also the testing x86-64 is not enough for such
changes (although I tried ppc64le and did not find any problem).
Cost related patches for RA frequently result in new testsuite failures
on some targets. Even if the change seems obvious and expected to
improve the generated code.
Target dependent code sometimes defines correctly the costs only for
some possible cases and making less dependent from this pitfall is
good. So I think the patch moves us to the right direction.
The patch is ok for me to commit it to the trunk after the gcc-13
release and if arm64 testing shows no GCC testsuite regression.
Thank you for working on this issue.
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH] Check hard_regno_mode_ok before setting lowest memory move cost for the mode with different reg classes.
2023-04-05 12:58 ` Vladimir Makarov
@ 2023-04-06 5:07 ` Liu, Hongtao
2023-04-19 5:53 ` Hongtao Liu
0 siblings, 1 reply; 5+ messages in thread
From: Liu, Hongtao @ 2023-04-06 5:07 UTC (permalink / raw)
To: Vladimir Makarov, Jeff Law, gcc-patches
> -----Original Message-----
> From: Vladimir Makarov <vmakarov@redhat.com>
> Sent: Wednesday, April 5, 2023 8:59 PM
> To: Jeff Law <jeffreyalaw@gmail.com>; Liu, Hongtao
> <hongtao.liu@intel.com>; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] Check hard_regno_mode_ok before setting lowest
> memory move cost for the mode with different reg classes.
>
>
> On 4/4/23 21:29, Jeff Law wrote:
> >
> >
> > On 4/3/23 23:13, liuhongt via Gcc-patches wrote:
> >> There's a potential performance issue when backend returns some
> >> unreasonable value for the mode which can be never be allocate with
> >> reg class.
> >>
> >> Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
> >> Ok for trunk(or GCC14 stage1)?
> >>
> >> gcc/ChangeLog:
> >>
> >> PR rtl-optimization/109351
> >> * ira.cc (setup_class_subset_and_memory_move_costs): Check
> >> hard_regno_mode_ok before setting lowest memory move cost for
> >> the mode with different reg classes.
> > Not a regression *and* changing register allocation. This seems like
> > it should defer to gcc-14.
> >
> Yes, I am agree. It should wait for gcc-14, especially when we are close to the
> release. Also the testing x86-64 is not enough for such changes (although I
> tried ppc64le and did not find any problem).
>
> Cost related patches for RA frequently result in new testsuite failures on
> some targets. Even if the change seems obvious and expected to improve
> the generated code.
>
> Target dependent code sometimes defines correctly the costs only for some
> possible cases and making less dependent from this pitfall is good. So I think
> the patch moves us to the right direction.
>
> The patch is ok for me to commit it to the trunk after the gcc-13 release and if
> arm64 testing shows no GCC testsuite regression.
Bootstrapped and regtested on aarch64-unknown-linux-gnu.
Waiting for GCC14.
>
> Thank you for working on this issue.
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Check hard_regno_mode_ok before setting lowest memory move cost for the mode with different reg classes.
2023-04-06 5:07 ` Liu, Hongtao
@ 2023-04-19 5:53 ` Hongtao Liu
0 siblings, 0 replies; 5+ messages in thread
From: Hongtao Liu @ 2023-04-19 5:53 UTC (permalink / raw)
To: Liu, Hongtao; +Cc: Vladimir Makarov, Jeff Law, gcc-patches
On Thu, Apr 6, 2023 at 1:07 PM Liu, Hongtao via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
>
> > -----Original Message-----
> > From: Vladimir Makarov <vmakarov@redhat.com>
> > Sent: Wednesday, April 5, 2023 8:59 PM
> > To: Jeff Law <jeffreyalaw@gmail.com>; Liu, Hongtao
> > <hongtao.liu@intel.com>; gcc-patches@gcc.gnu.org
> > Subject: Re: [PATCH] Check hard_regno_mode_ok before setting lowest
> > memory move cost for the mode with different reg classes.
> >
> >
> > On 4/4/23 21:29, Jeff Law wrote:
> > >
> > >
> > > On 4/3/23 23:13, liuhongt via Gcc-patches wrote:
> > >> There's a potential performance issue when backend returns some
> > >> unreasonable value for the mode which can be never be allocate with
> > >> reg class.
> > >>
> > >> Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
> > >> Ok for trunk(or GCC14 stage1)?
> > >>
> > >> gcc/ChangeLog:
> > >>
> > >> PR rtl-optimization/109351
> > >> * ira.cc (setup_class_subset_and_memory_move_costs): Check
> > >> hard_regno_mode_ok before setting lowest memory move cost for
> > >> the mode with different reg classes.
> > > Not a regression *and* changing register allocation. This seems like
> > > it should defer to gcc-14.
> > >
> > Yes, I am agree. It should wait for gcc-14, especially when we are close to the
> > release. Also the testing x86-64 is not enough for such changes (although I
> > tried ppc64le and did not find any problem).
> >
> > Cost related patches for RA frequently result in new testsuite failures on
> > some targets. Even if the change seems obvious and expected to improve
> > the generated code.
> >
> > Target dependent code sometimes defines correctly the costs only for some
> > possible cases and making less dependent from this pitfall is good. So I think
> > the patch moves us to the right direction.
> >
> > The patch is ok for me to commit it to the trunk after the gcc-13 release and if
> > arm64 testing shows no GCC testsuite regression.
> Bootstrapped and regtested on aarch64-unknown-linux-gnu.
> Waiting for GCC14.
Committed.
> >
> > Thank you for working on this issue.
> >
>
--
BR,
Hongtao
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-04-19 5:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-04 5:13 [PATCH] Check hard_regno_mode_ok before setting lowest memory move cost for the mode with different reg classes liuhongt
2023-04-05 1:29 ` Jeff Law
2023-04-05 12:58 ` Vladimir Makarov
2023-04-06 5:07 ` Liu, Hongtao
2023-04-19 5:53 ` Hongtao Liu
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).