Re: [PATCH v2 2/2] [PR100106] Reject unaligned subregs when strict alignment is required

[Alexandre Oliva <oliva@adacore.com>]

On May  5, 2022, Segher Boessenkool <segher@kernel.crashing.org> wrote:

> On Thu, May 05, 2022 at 08:59:21AM +0100, Richard Sandiford wrote:
>> Alexandre Oliva via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> I know this is the best being the enemy of the good, but given
>> that we're at the start of stage 1, would it be feasible to try
>> to get rid of (subreg (mem)) altogether for GCC 13?

> Yes please!

I'm not sure this is what you two had in mind, but the news I have is
not great.  With this patch, x86_64 has some regressions in vector
testcases (*), and ppc64le doesn't bootstrap (tsan_interface_atomic.o
ends up with a nil SET_DEST in split all insns).  aarch64 is still
building stage2.

I'm not sure this is enough.  IIRC register allocation modifies in place
pseudos that can't be assigned to hard registers, turning them into
MEMs.  If that's so, SUBREGs of such pseudos will silently become
SUBREGs of MEMs, and I don't know that they are validated again and, if
so, what happens to those that fail validation.

I kind of feel that this is more than I can tackle ATM, so I'd
appreciate if someone else would take this up and drive this transition.


Disallow SUBREG of MEM

Introduce TARGET_ALLOW_SUBREG_OF_MEM, defaulting to 0.

Reject SUBREG of MEM regardless of alignment, unless the macro is
defined to nonzero.


for  gcc/ChangeLog

	PR target/100106
	* emit-rtl.cc (validate_subreg) [!TARGET_ALLOW_SUBREG_OF_MEM]:
	Reject SUBREG of MEM.
---
 gcc/emit-rtl.cc |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/gcc/emit-rtl.cc b/gcc/emit-rtl.cc
index 9c03e27894fff..f055179b3b8a6 100644
--- a/gcc/emit-rtl.cc
+++ b/gcc/emit-rtl.cc
@@ -983,8 +983,12 @@ validate_subreg (machine_mode omode, machine_mode imode,
       return subreg_offset_representable_p (regno, imode, offset, omode);
     }
   /* Do not allow SUBREG with stricter alignment than the inner MEM.  */
-  else if (reg && MEM_P (reg) && STRICT_ALIGNMENT
-	   && MEM_ALIGN (reg) < GET_MODE_ALIGNMENT (omode))
+  else if (reg && MEM_P (reg)
+#if TARGET_ALLOW_SUBREG_OF_MEM /* ??? Reject them all eventually.  */
+	   && STRICT_ALIGNMENT
+	   && MEM_ALIGN (reg) < GET_MODE_ALIGNMENT (omode)
+#endif
+	   )
     return false;
 
   /* The outer size must be ordered wrt the register size, otherwise



(*) here are the x86_64 regressions introduced by the patch:

+ FAIL: gcc.target/i386/avx-2.c (internal compiler error: in gen_rtx_SUBREG, at emit-rtl.cc:1030)
+ FAIL: gcc.target/i386/avx-2.c (test for excess errors)
+ FAIL: gcc.target/i386/sse-14.c (internal compiler error: in gen_rtx_SUBREG, at emit-rtl.cc:1030)
+ FAIL: gcc.target/i386/sse-14.c (test for excess errors)
+ FAIL: gcc.target/i386/sse-22.c (internal compiler error: in gen_rtx_SUBREG, at emit-rtl.cc:1030)
+ FAIL: gcc.target/i386/sse-22.c (test for excess errors)
+ FAIL: gcc.target/i386/sse-22a.c (internal compiler error: in gen_rtx_SUBREG, at emit-rtl.cc:1030)
+ FAIL: gcc.target/i386/sse-22a.c (test for excess errors)

-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>