[v2] m68k: prevent moves with auto-inc/dec src overlapping with dst (PR123853, PR123076)

Message ID 20260707200922.820695-2-mikpelinux@gmail.com
State New
Headers
Series [v2] m68k: prevent moves with auto-inc/dec src overlapping with dst (PR123853, PR123076) |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap fail Build failed
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-aarch64-bootstrap fail Build failed

Commit Message

Mikael Pettersson July 7, 2026, 8:09 p.m. UTC
  Since gcc-15 combine can produce moves from an auto-inc/dec source to
a destination that uses the same register, causing wrong code on m68k.
Both glibc and Python are known to be miscompiled due to this.
This adjusts the m68k backend to reject such moves.

For glibc the difference is

-       move.l -(%a0),(%a0,%d0.l)
+       lea (%a0,%d0.l),%a1
+       move.l -(%a0),(%a1)

Python has a similar problem, but with a post-inc source operand.

Bootstrapped and regression tested on m68k-linux-gnu, no regressions.

Ok for trunk? And maybe gcc-16/15 after a week?

gcc/

	2026-07-07  Mikael Pettersson  <mikpelinux@gmail.com>

	PR rtl-optimization/123853
	PR target/123076
	* config/m68k/m68k-protos.h (check_move_simode): Declare.
	* config/m68k/m68k.cc (check_move_simode): New, reject move from
	auto-inc/dec source mem whose reg occurs in the destination address.
	* config/m68k/m68k.md (*movsi_m68k): Add check_move_simode to
	condition.
	(*movsi_m68k2): Likewise.
---
Changes since v1:
 - also reject POST_INC sources, required for Python build

 gcc/config/m68k/m68k-protos.h |  1 +
 gcc/config/m68k/m68k.cc       | 25 +++++++++++++++++++++++++
 gcc/config/m68k/m68k.md       |  4 ++--
 3 files changed, 28 insertions(+), 2 deletions(-)
  

Comments

Jeffrey Law July 7, 2026, 9:32 p.m. UTC | #1
On 7/7/2026 2:09 PM, Mikael Pettersson wrote:
> Since gcc-15 combine can produce moves from an auto-inc/dec source to
> a destination that uses the same register, causing wrong code on m68k.
> Both glibc and Python are known to be miscompiled due to this.
> This adjusts the m68k backend to reject such moves.
>
> For glibc the difference is
>
> -       move.l -(%a0),(%a0,%d0.l)
> +       lea (%a0,%d0.l),%a1
> +       move.l -(%a0),(%a1)
>
> Python has a similar problem, but with a post-inc source operand.
>
> Bootstrapped and regression tested on m68k-linux-gnu, no regressions.
>
> Ok for trunk? And maybe gcc-16/15 after a week?
>
> gcc/
>
> 	2026-07-07  Mikael Pettersson  <mikpelinux@gmail.com>
>
> 	PR rtl-optimization/123853
> 	PR target/123076
> 	* config/m68k/m68k-protos.h (check_move_simode): Declare.
> 	* config/m68k/m68k.cc (check_move_simode): New, reject move from
> 	auto-inc/dec source mem whose reg occurs in the destination address.
> 	* config/m68k/m68k.md (*movsi_m68k): Add check_move_simode to
> 	condition.
> 	(*movsi_m68k2): Likewise.
Sorry, no.  I think this is fundamentally the wrong way to go.  As I 
outlined elsewhere, the right way to go is the dynamic filtering.

Jeff
  

Patch

diff --git a/gcc/config/m68k/m68k-protos.h b/gcc/config/m68k/m68k-protos.h
index abb8e9b061d..248414eb167 100644
--- a/gcc/config/m68k/m68k-protos.h
+++ b/gcc/config/m68k/m68k-protos.h
@@ -26,6 +26,7 @@  extern HOST_WIDE_INT m68k_initial_elimination_offset (int from, int to);
 extern void split_di (rtx[], int, rtx[], rtx[]);
 
 extern bool valid_mov3q_const (HOST_WIDE_INT);
+extern bool check_move_simode (const rtx *);
 extern const char *output_move_simode (rtx *);
 extern const char *output_move_himode (rtx *);
 extern const char *output_move_qimode (rtx *);
diff --git a/gcc/config/m68k/m68k.cc b/gcc/config/m68k/m68k.cc
index b6134892883..dffc603d257 100644
--- a/gcc/config/m68k/m68k.cc
+++ b/gcc/config/m68k/m68k.cc
@@ -3227,6 +3227,31 @@  valid_mov3q_const (HOST_WIDE_INT i)
   return TARGET_ISAB && (i == -1 || IN_RANGE (i, 1, 7));
 }
 
+/* Return true if OPERANDS[] are valid for output_move_simode.
+   In particular, if OPERANDS[1] is a MEM with PRE_DEC or POST_INC
+   adressing and its REG is mentioned in OPERANDS[0], it's invalid.  */
+
+bool
+check_move_simode (const rtx *operands)
+{
+  rtx src = operands[1];
+  if (MEM_P (src))
+    {
+      rtx src1 = XEXP (src, 0);
+      if (GET_CODE (src1) == PRE_DEC || GET_CODE (src1) == POST_INC)
+	{
+	  rtx src2 = XEXP (src1, 0);
+	  if (REG_P (src2))
+	    {
+	      rtx dst = operands[0];
+	      if (reg_overlap_mentioned_p (src2, dst))
+		return false;
+	    }
+	}
+    }
+  return true;
+}
+
 /* Return an instruction to move CONST_INT OPERANDS[1] into OPERANDS[0].
    I is the value of OPERANDS[1].  */
 
diff --git a/gcc/config/m68k/m68k.md b/gcc/config/m68k/m68k.md
index e77ac13cf1b..f8b6a2f79dd 100644
--- a/gcc/config/m68k/m68k.md
+++ b/gcc/config/m68k/m68k.md
@@ -993,7 +993,7 @@ 
   ;; We don't allow f-regs since fixed point cannot go in them.
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g,d,a<")
         (match_operand:SI 1 "general_src_operand" "damSnT,n,i"))]
-  "!TARGET_COLDFIRE && reload_completed"
+  "!TARGET_COLDFIRE && reload_completed && check_move_simode (operands)"
 {
   return output_move_simode (operands);
 }
@@ -1006,7 +1006,7 @@ 
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g,d,a<")
         (match_operand:SI 1 "general_src_operand" "damSKT,n,i"))]
 
-  "!TARGET_COLDFIRE"
+  "!TARGET_COLDFIRE && check_move_simode (operands)"
 {
   return output_move_simode (operands);
 }