On Thu, Sep 9, 2021 at 1:09 PM Richard Earnshaw <[email protected]> wrote:
>
>
> gen_lowpart_general handles forming a SUBREG of a MEM by using
> adjust_address to rework and validate a new version of the MEM.
> However, gen_highpart does not attempt this and simply returns (SUBREG
> (MEM)) if the change is not 'obviously' safe. Improve on that by
> using a similar approach so that gen_lowpart and gen_highpart are
> mostly symmetrical in this regard.
When I decipher gen_lowpart correctly then it doesn't generate the
subreg of the mem in the first place so doing it like that in gen_highpart
would _not_ invoke simplify_gen_subreg on a MEM_P but instead
do what you now do directly?
I also wonder why gen_lowpart_general uses byte_lowpart_offset
while you use subreg_highpart_offset where subreg_lowpart_offset
is also available ... huh - and there's also
subreg_size_{lowpart,highpart}_offset.
So it looks like your case wouldn't handle the paradoxical highpart
(which better shouldn't be accessed?).
So like
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 77ea8948ee8..c3dae7d8075 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -1585,6 +1585,13 @@ gen_highpart (machine_mode mode, rtx x)
gcc_assert (known_le (msize, (unsigned int) UNITS_PER_WORD)
|| known_eq (msize, GET_MODE_UNIT_SIZE (GET_MODE (x))));
+ /* Offset MEMs. */
+ if (MEM_P (x))
+ {
+ poly_int64 offset = subreg_highpart_offset (mode, GET_MODE (x));
+ return adjust_address (x, mode, offset);
+ }
+
result = simplify_gen_subreg (mode, x, GET_MODE (x),
subreg_highpart_offset (mode, GET_MODE (x)));
gcc_assert (result);
Testing
+ else if (GET_CODE (result) == SUBREG && MEM_P (SUBREG_REG (result))
+ && MEM_P (x))
looks a bit odd to me.
I'll note it leaves gen_highpart_mode "unfixed", some refactoring should
instead commonize the worker for both interfaces, making gen_highpart
invoke gen_highpart_mode or so.
> gcc/ChangeLog:
>
> PR target/102125
> * emit-rtl.c (gen_highpart): If simplify_gen_subreg returns
> SUBREG (MEM) for a MEM, use adjust_address to produce a new
> MEM.
> ---
> gcc/emit-rtl.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
