"Robin Dapp" <[email protected]> writes:
> Grml, setting REGMODE_NATURAL_SIZE for advsimd modes as well uncovered 
> several 
> other lower-subreg issues and it, again, turned into a game of whack-a-mole 
> and 
> obviously took way longer than I would have hoped.

Yeah, the aarch64 change looks good to me (but I shouldn't approve it).

And sorry for the late reply.  Since like you said the lower-subreg
changes came from whack-a-mole, I kind-of needed to play the same game
to review.

An aarch64 bootstrap & regression test succeeded for me except for:

FAIL: gcc.target/aarch64/simd/fold_to_highpart_6.c check-function-bodies 
test_256b_type

with:

1. the read_complex_part change (already approved)
2. the expemd.cc changes (already approved)
3. the interesting_mode_p change
4. the change below
5. the aarch64 change

Do (3) and (4) work for you too, or do you still need some of the other
lower-subreg changes from your patch?  Could you give an example if so?

> @@ -4714,6 +4711,22 @@ emit_move_insn (rtx x, rtx y)
>        mode = GET_MODE (x_inner);
>      }
>  
> +  /* If we don't have a move for MODE, emit_move_insn_1 will fall back to
> +     emit_move_multi_word at which point we'll try to decompose the
> +     inner reg via operand_subword_force.  If that fails we're back
> +     here again without having made progress.
> +     Therefore, spill Y to memory before we cannot escape anymore.  */
> +  rtx yreg = SUBREG_P (y) ? SUBREG_REG (y) : y;
> +  if (optab_handler (mov_optab, mode) == CODE_FOR_nothing
> +        && maybe_lt ((unsigned) UNITS_PER_WORD,
> +                     (poly_uint64) REGMODE_NATURAL_SIZE (GET_MODE (yreg))))
> +    {
> +      rtx mem = assign_stack_temp (GET_MODE (yreg),
> +                                GET_MODE_SIZE (GET_MODE (yreg)));
> +      emit_move_insn (mem, yreg);
> +      y = adjust_address (mem, mode, 0);

The 0 would need to be subreg_memory_offset (y) when y is a subreg.
But this might still go wrong for paradoxical subregs, where we could
read beyond the stack temporary.

Could you list some of the testcases that fail for RISC-V without the
emit_move_insn change?  (Maybe you already did, sorry.)

I won't be able to reply for a week or so.  I'll have a look at
fold_to_highpart_6.c when I get back.

Thanks,
Richard


diff --git a/gcc/lower-subreg.cc b/gcc/lower-subreg.cc
index 5dee6a0b646..3d0654e7d93 100644
--- a/gcc/lower-subreg.cc
+++ b/gcc/lower-subreg.cc
@@ -302,7 +304,21 @@ static bool
 simple_move_operand (rtx x)
 {
   if (GET_CODE (x) == SUBREG)
-    x = SUBREG_REG (x);
+    {
+      /* Exclude subregs whose outer mode can be split into multiple words
+        but whose inner mode cannot.  Attempting to split such a subreg
+        would mean trying to split the unsplittable inner register.
+
+        If instead the subreg occupies a single word, we can keep it as-is,
+        regardless of what the SUBREG_REG is.  If the outer mode cannot be
+        split then the subreg makes things no worse than they already are.  */
+      unsigned int factor, size;
+      if (interesting_mode_p (GET_MODE (x), &size, &factor)
+         && factor > 1
+         && !interesting_mode_p (GET_MODE (SUBREG_REG (x)), &size, &factor))
+       return false;
+      x = SUBREG_REG (x);
+    }
 
   if (!OBJECT_P (x))
     return false;

Reply via email to