m68k: prevent moves with pre-dec src overlapping with dst (PR123853)

Message ID 20260707104821.64917-2-mikpelinux@gmail.com
State New
Headers
Series m68k: prevent moves with pre-dec src overlapping with dst (PR123853) |

Checks

Context Check Description
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_gcc_check--master-arm pending Build did not complete; will be retriggered
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap pending Build did not complete; will be retriggered

Commit Message

Mikael Pettersson July 7, 2026, 10:47 a.m. UTC
  Since gcc-15 combine can produce moves from a PRE_DEC source to a
destination that uses the same register, causing wrong code on m68k.
In particular the glibc build is broken.  This adjusts the m68k
backend to reject such moves:

        move.l -(%a0),(%a0,%d0.l)

and instead emit:

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

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
	* config/m68k/m68k-protos.h (check_move_simode): Declare.
	* config/m68k/m68k.cc (check_move_simode): New, reject moves from
	a pre-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.
---
 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, 12:02 p.m. UTC | #1
On 7/7/2026 4:47 AM, Mikael Pettersson wrote:
> Since gcc-15 combine can produce moves from a PRE_DEC source to a
> destination that uses the same register, causing wrong code on m68k.
> In particular the glibc build is broken.  This adjusts the m68k
> backend to reject such moves:
>
>          move.l -(%a0),(%a0,%d0.l)
>
> and instead emit:
>
>          lea (%a0,%d0.l),%a1
>          move.l -(%a0),(%a1)
>
> 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
> 	* config/m68k/m68k-protos.h (check_move_simode): Declare.
> 	* config/m68k/m68k.cc (check_move_simode): New, reject moves from
> 	a pre-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.
I think the new dynamic register filters are going to be the way to 
handle this.  The core issue is those additional uses of a 
auto-incremented register, which requires looking at two operands to 
determine if the insn matches its constraints.  This patch just papers 
over the problem and it likely triggers elsewhere in the port with some 
effort.

My recommendation is to wait until Pan Li's work to utilize the dynamic 
register filtering lands in the RISC-V port across the board and gets a 
time to shake out any additional LRA gotchas, then use it on the m68k, 
h8 and pdp11 which all have the same core problem.

Jeff
  
Paul Koning July 7, 2026, 1:31 p.m. UTC | #2
> On Jul 7, 2026, at 8:02 AM, Jeffrey Law <jeffrey.law@oss.qualcomm.com> wrote:
> 
> 
> 
> On 7/7/2026 4:47 AM, Mikael Pettersson wrote:
>> Since gcc-15 combine can produce moves from a PRE_DEC source to a
>> destination that uses the same register, causing wrong code on m68k.
>> In particular the glibc build is broken.  This adjusts the m68k
>> backend to reject such moves:
>> 
>>         move.l -(%a0),(%a0,%d0.l)
>> 
>> and instead emit:
>> 
>>         lea (%a0,%d0.l),%a1
>>         move.l -(%a0),(%a1)
>> 
>> 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
>> * config/m68k/m68k-protos.h (check_move_simode): Declare.
>> * config/m68k/m68k.cc (check_move_simode): New, reject moves from
>> a pre-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.
> I think the new dynamic register filters are going to be the way to handle this.  The core issue is those additional uses of a auto-incremented register, which requires looking at two operands to determine if the insn matches its constraints.  This patch just papers over the problem and it likely triggers elsewhere in the port with some effort.
> 
> My recommendation is to wait until Pan Li's work to utilize the dynamic register filtering lands in the RISC-V port across the board and gets a time to shake out any additional LRA gotchas, then use it on the m68k, h8 and pdp11 which all have the same core problem.

I dealt with it in pdp11.md in a different way, using a string of constraints to express this.  It seems to work but it's rather cumbersome.

I didn't really understand how the patch addresses the issue.  My thought was that we're dealing with a constraint here, so expressing it as such seemed logical.  But this patch certainly is simpler.

	paul
  
Jeffrey Law July 7, 2026, 2:21 p.m. UTC | #3
On 7/7/2026 7:31 AM, Paul Koning wrote:
>
>> On Jul 7, 2026, at 8:02 AM, Jeffrey Law <jeffrey.law@oss.qualcomm.com> wrote:
>>
>>
>>
>> On 7/7/2026 4:47 AM, Mikael Pettersson wrote:
>>> Since gcc-15 combine can produce moves from a PRE_DEC source to a
>>> destination that uses the same register, causing wrong code on m68k.
>>> In particular the glibc build is broken.  This adjusts the m68k
>>> backend to reject such moves:
>>>
>>>          move.l -(%a0),(%a0,%d0.l)
>>>
>>> and instead emit:
>>>
>>>          lea (%a0,%d0.l),%a1
>>>          move.l -(%a0),(%a1)
>>>
>>> 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
>>> * config/m68k/m68k-protos.h (check_move_simode): Declare.
>>> * config/m68k/m68k.cc (check_move_simode): New, reject moves from
>>> a pre-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.
>> I think the new dynamic register filters are going to be the way to handle this.  The core issue is those additional uses of a auto-incremented register, which requires looking at two operands to determine if the insn matches its constraints.  This patch just papers over the problem and it likely triggers elsewhere in the port with some effort.
>>
>> My recommendation is to wait until Pan Li's work to utilize the dynamic register filtering lands in the RISC-V port across the board and gets a time to shake out any additional LRA gotchas, then use it on the m68k, h8 and pdp11 which all have the same core problem.
> I dealt with it in pdp11.md in a different way, using a string of constraints to express this.  It seems to work but it's rather cumbersome.
Yea.  I took your idea and implemented it for the H8.  As you note, it's 
cumbersome and as the number of registers you have to deal with grows, 
it gets worse.  When we started looking at the overlap problems with 
vector on RISC-V it was pretty clear that this approach wouldn't be 
maintainable.  The design of the dynamic filters was mean to handle the 
RISC-V case, but handling the m68k, h8 and pdp11 cases was always in the 
back of my mind.

Jeff
  
Paul Koning July 7, 2026, 5:22 p.m. UTC | #4
> On Jul 7, 2026, at 10:21 AM, Jeffrey Law <jeffrey.law@oss.qualcomm.com> wrote:
> 
> 
> 
> On 7/7/2026 7:31 AM, Paul Koning wrote:
>> 
>>> On Jul 7, 2026, at 8:02 AM, Jeffrey Law <jeffrey.law@oss.qualcomm.com> wrote:
>>> 
>>> 
>>> 
>>> On 7/7/2026 4:47 AM, Mikael Pettersson wrote:
>>>> Since gcc-15 combine can produce moves from a PRE_DEC source to a
>>>> destination that uses the same register, causing wrong code on m68k.
>>>> In particular the glibc build is broken.  This adjusts the m68k
>>>> backend to reject such moves:
>>>> 
>>>>         move.l -(%a0),(%a0,%d0.l)
>>>> 
>>>> and instead emit:
>>>> 
>>>>         lea (%a0,%d0.l),%a1
>>>>         move.l -(%a0),(%a1)
>>>> 
>>>> 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
>>>> * config/m68k/m68k-protos.h (check_move_simode): Declare.
>>>> * config/m68k/m68k.cc (check_move_simode): New, reject moves from
>>>> a pre-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.
>>> I think the new dynamic register filters are going to be the way to handle this.  The core issue is those additional uses of a auto-incremented register, which requires looking at two operands to determine if the insn matches its constraints.  This patch just papers over the problem and it likely triggers elsewhere in the port with some effort.
>>> 
>>> My recommendation is to wait until Pan Li's work to utilize the dynamic register filtering lands in the RISC-V port across the board and gets a time to shake out any additional LRA gotchas, then use it on the m68k, h8 and pdp11 which all have the same core problem.
>> I dealt with it in pdp11.md in a different way, using a string of constraints to express this.  It seems to work but it's rather cumbersome.
> Yea.  I took your idea and implemented it for the H8.  As you note, it's cumbersome and as the number of registers you have to deal with grows, it gets worse.  When we started looking at the overlap problems with vector on RISC-V it was pretty clear that this approach wouldn't be maintainable.  The design of the dynamic filters was mean to handle the RISC-V case, but handling the m68k, h8 and pdp11 cases was always in the back of my mind.

That sounds great.  Is there a pointer to a description of this approach?  I may want to switch to that, it sure would make things more readable.

	paul
  
Sam James July 7, 2026, 5:39 p.m. UTC | #5
Jeffrey Law <jeffrey.law@oss.qualcomm.com> writes:

> On 7/7/2026 4:47 AM, Mikael Pettersson wrote:
>> Since gcc-15 combine can produce moves from a PRE_DEC source to a
>> destination that uses the same register, causing wrong code on m68k.
>> In particular the glibc build is broken.  This adjusts the m68k
>> backend to reject such moves:
>>
>>          move.l -(%a0),(%a0,%d0.l)
>>
>> and instead emit:
>>
>>          lea (%a0,%d0.l),%a1
>>          move.l -(%a0),(%a1)
>>
>> 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
>> 	* config/m68k/m68k-protos.h (check_move_simode): Declare.
>> 	* config/m68k/m68k.cc (check_move_simode): New, reject moves from
>> 	a pre-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.
> I think the new dynamic register filters are going to be the way to
> handle this.  The core issue is those additional uses of a
> auto-incremented register, which requires looking at two operands to
> determine if the insn matches its constraints.  This patch just papers
> over the problem and it likely triggers elsewhere in the port with
> some effort.
>
> My recommendation is to wait until Pan Li's work to utilize the
> dynamic register filtering lands in the RISC-V port across the board
> and gets a time to shake out any additional LRA gotchas, then use it
> on the m68k, h8 and pdp11 which all have the same core problem.

For the PR, we'd love something backportable though, and having a patch
which was never on trunk isn't ideal, though it is quite targeted..

>
> Jeff
  
Mikael Pettersson July 7, 2026, 8:07 p.m. UTC | #6
On Tue, Jul 7, 2026 at 2:02 PM Jeffrey Law <jeffrey.law@oss.qualcomm.com> wrote:
>
>
>
> On 7/7/2026 4:47 AM, Mikael Pettersson wrote:
> > Since gcc-15 combine can produce moves from a PRE_DEC source to a
> > destination that uses the same register, causing wrong code on m68k.
> > In particular the glibc build is broken.  This adjusts the m68k
> > backend to reject such moves:
> >
> >          move.l -(%a0),(%a0,%d0.l)
> >
> > and instead emit:
> >
> >          lea (%a0,%d0.l),%a1
> >          move.l -(%a0),(%a1)
> >
> > 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
> >       * config/m68k/m68k-protos.h (check_move_simode): Declare.
> >       * config/m68k/m68k.cc (check_move_simode): New, reject moves from
> >       a pre-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.
> I think the new dynamic register filters are going to be the way to
> handle this.  The core issue is those additional uses of a
> auto-incremented register, which requires looking at two operands to
> determine if the insn matches its constraints.  This patch just papers
> over the problem and it likely triggers elsewhere in the port with some
> effort.
>
> My recommendation is to wait until Pan Li's work to utilize the dynamic
> register filtering lands in the RISC-V port across the board and gets a
> time to shake out any additional LRA gotchas, then use it on the m68k,
> h8 and pdp11 which all have the same core problem.

I can understand wanting to wait for a cleaner solution, but that won't
help users of gcc-15 and 16 that still have this wrong-code bug.

I'll post a v2 shortly that also fixes a similar issue when building Python.

/Mikael
  
Jeffrey Law July 8, 2026, 5:46 a.m. UTC | #7
On 7/7/2026 11:39 AM, Sam James wrote:
> For the PR, we'd love something backportable though, and having a patch
> which was never on trunk isn't ideal, though it is quite targeted..
I certainly realize the issues here with wanting something 
backportable.  But this may be one we just have to live with.

jeff
  
Oleg Endo July 8, 2026, 5:56 a.m. UTC | #8
On Tue, 2026-07-07 at 23:46 -0600, Jeffrey Law wrote:
> 
> On 7/7/2026 11:39 AM, Sam James wrote:
> > For the PR, we'd love something backportable though, and having a patch
> > which was never on trunk isn't ideal, though it is quite targeted..
> I certainly realize the issues here with wanting something 
> backportable.  But this may be one we just have to live with.
> 

What's the problem with committing & backport the hacky fix now and then,
once the infra for the more ideal solution lands, revert the commit and re-
do it on master?

At least that would give m68k compiler users access to a recent GCC 15 and
GCC 16 today.

Best regards,
Oleg Endo
  
Jeffrey Law July 8, 2026, 6:05 a.m. UTC | #9
On 7/7/2026 11:56 PM, Oleg Endo wrote:
> On Tue, 2026-07-07 at 23:46 -0600, Jeffrey Law wrote:
>> On 7/7/2026 11:39 AM, Sam James wrote:
>>> For the PR, we'd love something backportable though, and having a patch
>>> which was never on trunk isn't ideal, though it is quite targeted..
>> I certainly realize the issues here with wanting something
>> backportable.  But this may be one we just have to live with.
>>
> What's the problem with committing & backport the hacky fix now and then,
> once the infra for the more ideal solution lands, revert the commit and re-
> do it on master?
>
> At least that would give m68k compiler users access to a recent GCC 15 and
> GCC 16 today.
Because it's not actually a fix.  This problem can show up in many other 
places and it's an unmaintainable hack.

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..e95b83f5e18 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 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)
+	{
+	  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);
 }