[gcc(refs/vendors/riscv/heads/gcc-15-with-riscv-opts)] [committed] [PR rtl-optimization/120242] Fix SUBREG_PROMOTED_VAR_P after ext-dce's actions

2025-07-05 Thread Jeff Law via Gcc-cvs
https://gcc.gnu.org/g:102e91566d6f90ecd52f50c4a340d116dc8b1f1a

commit 102e91566d6f90ecd52f50c4a340d116dc8b1f1a
Author: Jeff Law 
Date:   Mon Jun 30 14:38:33 2025 -0600

[committed] [PR rtl-optimization/120242] Fix SUBREG_PROMOTED_VAR_P after 
ext-dce's actions

I've gone back and forth of these problems multiple times.  We have two 
passes,
ext-dce and combine which eliminate extensions using totally different
mechanisms.

ext-dce looks for cases where the state of upper bits in an object aren't
observable and if they aren't observable, then eliminates extensions which 
set
those bits.

combine looks for cases where we know the state of the upper bits and can 
prove
an extension is just setting those bits to their prior value.  Combine also
looks for cases where the precise extension isn't really important, just the
knowledge that the upper bits are zero or sign extended from a narrower mode
is needed.

Combine relies heavily on the SUBREG_PROMOTED_VAR state to do its job.  If 
the
actions of ext-dce (or any other pass for that matter) make
SUBREG_PROMOTED_VAR's state inconsistent with combine's expectations, then
combine can end up generating incorrect code.

--

When ext-dce eliminates an extension and turns it into a subreg copy 
(without
any known SUBREG_PROMOTED_VAR state).  Since we can no longer guarantee the
destination object has any known extension state, we scurry around and wipe
SUBREG_PROMOTED_VAR state for the destination object.

That's fine and dandy, but ultimately insufficient.  Consider if the
destination of the optimized extension was used as a source in a simple copy
insn.  Furthermore assume that the destination of that copy is used within a
SUBREG expression with SUBREG_PROMOTED_VAR set.  ext-dce's actions have
clobbered the SUBREG_PROMOTED_VAR state on the destination of that copy, 
albeit
indirectly.

This patch addresses this problem by taking the set of pseudos directly
impacted by ext-dce's actions and expands that set by building a transitive
closure for pseudos connected via copies.  We then scurry around finding
SUBREG_PROMOTED_VAR state to wipe for everything in that expanded set of
pseudos.  Voila, everything just works.

--

The other approach here would be to further expand the liveness sets inside
ext-dce.  That's a simpler path forward, but ultimately regresses the 
quality
of codes we do care about.

One good piece of news is that with the transitive closure bits in place, we
can eliminate a bit of the live set expansion we had in place for
SUBREG_PROMOTED_VAR objects.

--

So let's take one case of the 5 that have been reported.

In ext-dce we have this insn:

> (insn 29 27 30 3 (set (reg:DI 134 [ al_lsm.9 ])
> (zero_extend:DI (subreg:HI (reg:DI 162) 0))) "j.c":17:17 552 
{*zero_extendhidi2_bitmanip}
>  (expr_list:REG_DEAD (reg:DI 162)
> (nil)))

There are reachable uses of (reg 134):

> (insn 49 47 52 6 (set (mem/c:HI (lo_sum:DI (reg/f:DI 186)
> (symbol_ref:DI ("al") [flags 0x86]  )) [2 al+0 S2 A16])
> (subreg/s/v:HI (reg:DI 134 [ al_lsm.9 ]) 0)) 279 {*movhi_internal}
>  (expr_list:REG_DEAD (reg/f:DI 186)
> (nil)))Obviously safe if we were to remove the extension.

> (insn 52 49 53 6 (set (reg:DI 176)
> (and:DI (reg:DI 134 [ al_lsm.9 ])
> (const_int 5 [0x5]))) "j.c":21:12 106 {*anddi3}
>  (expr_list:REG_DEAD (reg:DI 134 [ al_lsm.9 ])
> (nil)))
> (insn 53 52 56 6 (set (reg:SI 177 [ _8 ])
> (zero_extend:SI (subreg:HI (reg:DI 176) 0))) "j.c":21:12 551 
{*zero_extendhisi2_bitmanip}
>  (expr_list:REG_DEAD (reg:DI 176)
> (nil))) Safe to remove the extension as we only read the low 16 
bits from the destination register (reg 176) in insn 53.

> (insn 27 26 29 3 (set (reg:DI 162)
> (sign_extend:DI (plus:SI (subreg/s/v:SI (reg:DI 134 [ al_lsm.9 ]) 
0)
> (const_int 1 [0x1] "j.c":17:17 8 {addsi3_extended}
>  (expr_list:REG_DEAD (reg:DI 134 [ al_lsm.9 ])
> (nil)))
> (insn 29 27 30 3 (set (reg:DI 134 [ al_lsm.9 ])
> (zero_extend:DI (subreg:HI (reg:DI 162) 0))) "j.c":17:17 552 
{*zero_extendhidi2_bitmanip}
>  (expr_list:REG_DEAD (reg:DI 162)
> (nil)))

Again, not as obvious as the first case, but we only read the low 16 bits 
from
(reg 162) in insn 29.  So those upper bits in (reg 134) don't matter.

> (insn 26 92 27 3 (set (reg:DI 144 [ ivtmp.17 ])
> (reg:DI 134 [ al_lsm.9 ])) 277 {*movdi_64bit}
>  (nil))
> (insn 30 29 31 3 (set (reg:DI 135 [ al.2_3 ])
> (sign_extend:DI (subreg/s/v:HI (reg:DI 144 [ ivtmp.17 ])

[gcc(refs/vendors/riscv/heads/gcc-15-with-riscv-opts)] [committed] [PR rtl-optimization/120242] Fix SUBREG_PROMOTED_VAR_P after ext-dce's actions

2025-07-03 Thread Jeff Law via Gcc-cvs
https://gcc.gnu.org/g:c09ee710d2fe89169946e1d2909d9fa27da47b8b

commit c09ee710d2fe89169946e1d2909d9fa27da47b8b
Author: Jeff Law 
Date:   Mon Jun 30 14:38:33 2025 -0600

[committed] [PR rtl-optimization/120242] Fix SUBREG_PROMOTED_VAR_P after 
ext-dce's actions

I've gone back and forth of these problems multiple times.  We have two 
passes,
ext-dce and combine which eliminate extensions using totally different
mechanisms.

ext-dce looks for cases where the state of upper bits in an object aren't
observable and if they aren't observable, then eliminates extensions which 
set
those bits.

combine looks for cases where we know the state of the upper bits and can 
prove
an extension is just setting those bits to their prior value.  Combine also
looks for cases where the precise extension isn't really important, just the
knowledge that the upper bits are zero or sign extended from a narrower mode
is needed.

Combine relies heavily on the SUBREG_PROMOTED_VAR state to do its job.  If 
the
actions of ext-dce (or any other pass for that matter) make
SUBREG_PROMOTED_VAR's state inconsistent with combine's expectations, then
combine can end up generating incorrect code.

--

When ext-dce eliminates an extension and turns it into a subreg copy 
(without
any known SUBREG_PROMOTED_VAR state).  Since we can no longer guarantee the
destination object has any known extension state, we scurry around and wipe
SUBREG_PROMOTED_VAR state for the destination object.

That's fine and dandy, but ultimately insufficient.  Consider if the
destination of the optimized extension was used as a source in a simple copy
insn.  Furthermore assume that the destination of that copy is used within a
SUBREG expression with SUBREG_PROMOTED_VAR set.  ext-dce's actions have
clobbered the SUBREG_PROMOTED_VAR state on the destination of that copy, 
albeit
indirectly.

This patch addresses this problem by taking the set of pseudos directly
impacted by ext-dce's actions and expands that set by building a transitive
closure for pseudos connected via copies.  We then scurry around finding
SUBREG_PROMOTED_VAR state to wipe for everything in that expanded set of
pseudos.  Voila, everything just works.

--

The other approach here would be to further expand the liveness sets inside
ext-dce.  That's a simpler path forward, but ultimately regresses the 
quality
of codes we do care about.

One good piece of news is that with the transitive closure bits in place, we
can eliminate a bit of the live set expansion we had in place for
SUBREG_PROMOTED_VAR objects.

--

So let's take one case of the 5 that have been reported.

In ext-dce we have this insn:

> (insn 29 27 30 3 (set (reg:DI 134 [ al_lsm.9 ])
> (zero_extend:DI (subreg:HI (reg:DI 162) 0))) "j.c":17:17 552 
{*zero_extendhidi2_bitmanip}
>  (expr_list:REG_DEAD (reg:DI 162)
> (nil)))

There are reachable uses of (reg 134):

> (insn 49 47 52 6 (set (mem/c:HI (lo_sum:DI (reg/f:DI 186)
> (symbol_ref:DI ("al") [flags 0x86]  )) [2 al+0 S2 A16])
> (subreg/s/v:HI (reg:DI 134 [ al_lsm.9 ]) 0)) 279 {*movhi_internal}
>  (expr_list:REG_DEAD (reg/f:DI 186)
> (nil)))Obviously safe if we were to remove the extension.

> (insn 52 49 53 6 (set (reg:DI 176)
> (and:DI (reg:DI 134 [ al_lsm.9 ])
> (const_int 5 [0x5]))) "j.c":21:12 106 {*anddi3}
>  (expr_list:REG_DEAD (reg:DI 134 [ al_lsm.9 ])
> (nil)))
> (insn 53 52 56 6 (set (reg:SI 177 [ _8 ])
> (zero_extend:SI (subreg:HI (reg:DI 176) 0))) "j.c":21:12 551 
{*zero_extendhisi2_bitmanip}
>  (expr_list:REG_DEAD (reg:DI 176)
> (nil))) Safe to remove the extension as we only read the low 16 
bits from the destination register (reg 176) in insn 53.

> (insn 27 26 29 3 (set (reg:DI 162)
> (sign_extend:DI (plus:SI (subreg/s/v:SI (reg:DI 134 [ al_lsm.9 ]) 
0)
> (const_int 1 [0x1] "j.c":17:17 8 {addsi3_extended}
>  (expr_list:REG_DEAD (reg:DI 134 [ al_lsm.9 ])
> (nil)))
> (insn 29 27 30 3 (set (reg:DI 134 [ al_lsm.9 ])
> (zero_extend:DI (subreg:HI (reg:DI 162) 0))) "j.c":17:17 552 
{*zero_extendhidi2_bitmanip}
>  (expr_list:REG_DEAD (reg:DI 162)
> (nil)))

Again, not as obvious as the first case, but we only read the low 16 bits 
from
(reg 162) in insn 29.  So those upper bits in (reg 134) don't matter.

> (insn 26 92 27 3 (set (reg:DI 144 [ ivtmp.17 ])
> (reg:DI 134 [ al_lsm.9 ])) 277 {*movdi_64bit}
>  (nil))
> (insn 30 29 31 3 (set (reg:DI 135 [ al.2_3 ])
> (sign_extend:DI (subreg/s/v:HI (reg:DI 144 [ ivtmp.17 ])