On 17/12/25 2:18 pm, Andrew Pinski wrote:
> Note you have the same issue with the following C example before this
> patch (just after you get with the original form too):
> ```
> unsigned foo0 (unsigned x, unsigned y)
> {
> unsigned x1 = x >> 31;
> unsigned yy = y & ~1;
> return x1 | yy;
> }
> ```
>
> That was something I was trying to point out.
> And why having the check for INTEGER_CST is just exchanging one issue
> for another. It does not solve the new issue here.
>
> Wait, I think we are losing/not using the SUBREG_PROMOTED_UNSIGNED_P
> fact here which is causing the issues really.
> We lost it when we did the original 2->7 combine (for foo0 above):
> ```
> Trying 2 -> 7:
> 2: r121:DI=r128:DI
> REG_DEAD r128:DI
> REG_EQUIV [ap:DI+0x30]
> 7: r124:SI=r121:DI#4 0>>0x1f
> REG_DEAD r121:DI
> Failed to match this instruction:
> (set (subreg:DI (reg:SI 124 [ x1_2 ]) 0)
> (zero_extract:DI (reg:DI 128 [ x+-4 ])
> (const_int 32 [0x20])
> (const_int 1 [0x1])))
> Successfully matched this instruction:
> (set (subreg:DI (reg:SI 124 [ x1_2 ]) 0)
> (and:DI (lshiftrt:DI (reg:DI 128 [ x+-4 ])
> (const_int 31 [0x1f]))
> (const_int 4294967295 [0xffffffff])))
> ```
>
> insn 7 was:
> ```
> (insn 7 4 9 2 (set (reg:SI 124 [ x1_2 ])
> (lshiftrt:SI (subreg/s/v:SI (reg/v:DI 121 [ x+-4 ]) 4)
> (const_int 31 [0x1f]))) "/app/example.cpp":30:12 292 {lshrsi3}
> (expr_list:REG_DEAD (reg/v:DI 121 [ x+-4 ])
> (nil)))
> ```
> insn 2 was just move.
> Note /s/v on the subreg there.
> If we didn't lose the SUBREG_PROMOTED_UNSIGNED_P, then things would just work.
> So you need to figure out how to get
>
> So again the INTEGER_CST is just working around an issue down the line
> and not really a good idea; in the case of ppc, the losing of the
> SUBREG_PROMOTED_UNSIGNED_P.
>
> Thanks,
> Andrew Pinski
Hello Andrew and Segher,
I went through combine pass traces, and the loss of the
SUBREG_PROMOTED_UNSIGNED_P flag is during simplification.
The issue specifically happens in simplify_shift_const.
It attempts to commute the shift with the subreg to "widen" the operation to
the native mode (DI),
but in doing so, it destroys the metadata.
Input (SImode): (lshiftrt:SI (subreg/s/v:SI (reg:DI 128 [ x ]) 0) (const_int
31))
Output (DImode Widened): (subreg:SI (lshiftrt:DI (reg:DI 128 [ x ]) (const_int
31)) 0)
Let’s say, somehow we manage to keep SUBREG_PROMOTED_UNSIGNED_P after
simplify_shift_const(modify to preserve /s/v flags),
and let us assume we have expression
(set (reg:SI 124 [ x1_2 ])
(subreg:SI/v/s (lshiftrt:DI (reg:DI 128 [ x ])
(const_int 31 [0x1f])) 0))
But in later simplification, when gcc tries to simplify set.. It tries to
create compund expression from set via make_compound_operation.
it typically SUBREG_REG(x) (the inner lshiftrt:DI) and passes that to helper
functions.
Functions like gen_lowpart, force_to_mode, or gen_lowpart_or_truncate then
re-wrap the result in a fresh subreg (truncation), losing the flags again.
Furthermore, even if we preserve the flag, combine tends to generate a
ZERO_EXTRACT pattern:
(set (subreg/s/v:DI (reg:SI 124) 0)
(zero_extract:DI (reg:DI 128) (const_int 32) (const_int 1)))
The above result we need to convert again back to
(set (reg:SI 124 [ x1_2 ])
(subreg:SI/s/v (zero_extract:DI (reg:DI 128 [ x ])
(const_int 32 [0x20])
(const_int 1 [0x1])) 0))
Finally, change_zero_ext would need to detect this specific pattern and
simplify it back to the optimal shift form:
(set (reg:SI 124) (lshiftrt:SI (subreg/s/v:SI (reg:DI 128 [ x ]) 0)
(const_int 31 [0x1f])))
Preserving the SUBREG_PROMOTED_UNSIGNED_P property is difficult because it
requires
updating the resulting SUBREG after every invocation of gen_lowpart,
force_to_mode, or gen_lowpart_or_truncate.
So, My question here is should we go with preserve and restore the flag
throughout the pipeline,
or should we just avoid widening subregs with SUBREG_PROMOTED_UNSIGNED_P?
Thanks,
Kishan
