builtins.c: Ensure emit_move_insn operands are valid (PR100418)

Message ID 6d405e80-365c-7e62-e36a-b6017a83795d@codesourcery.com
State New
Headers show
Series
  • builtins.c: Ensure emit_move_insn operands are valid (PR100418)
Related show

Commit Message

Andrew Stubbs May 7, 2021, 4:26 p.m.
A recent patch from Alexandre added new calls to emit_move_insn with 
PLUS expressions in the operands. Apparently this works fine on (at 
least) x86_64, but fails on (at least) amdgcn, where the adddi3 patten 
has clobbers that the movdi3 does not. This results in ICEs in recog.

This patch inserts force_operand around the problem cases so that it 
only creates valid move instructions.

I've done a regression test on amdgcn and everything works again [*].

OK to commit?

Andrew

[*] Well, once I fix a new, unrelated TImode issue it does anyway.
Ensure emit_move_insn operands are valid

Some architectures are fine with PLUS in move instructions, but others
are not (amdgcn is the motivating example).

gcc/ChangeLog:

	PR target/100418
	* builtins.c (try_store_by_multiple_pieces): Use force_operand for
	emit_move_insn operands.

Comments

Martin Sebor via Gcc-patches May 7, 2021, 6:12 p.m. | #1
On 5/7/2021 10:26 AM, Andrew Stubbs wrote:
> A recent patch from Alexandre added new calls to emit_move_insn with 

> PLUS expressions in the operands. Apparently this works fine on (at 

> least) x86_64, but fails on (at least) amdgcn, where the adddi3 patten 

> has clobbers that the movdi3 does not. This results in ICEs in recog.

>

> This patch inserts force_operand around the problem cases so that it 

> only creates valid move instructions.

>

> I've done a regression test on amdgcn and everything works again [*].

>

> OK to commit?

>

> Andrew

>

> [*] Well, once I fix a new, unrelated TImode issue it does anyway.

>

> 210507-fix-try-store.patch

>

> Ensure emit_move_insn operands are valid

>

> Some architectures are fine with PLUS in move instructions, but others

> are not (amdgcn is the motivating example).

>

> gcc/ChangeLog:

>

> 	PR target/100418

> 	* builtins.c (try_store_by_multiple_pieces): Use force_operand for

> 	emit_move_insn operands.


OK.  I've had the equivalent here, but hadn't submitted it yet.

jeff

Patch

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 0db4090c434..ef8852418af 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -6773,9 +6773,10 @@  try_store_by_multiple_pieces (rtx to, rtx len, unsigned int ctz_len,
 
       /* Adjust PTR, TO and REM.  Since TO's address is likely
 	 PTR+offset, we have to replace it.  */
-      emit_move_insn (ptr, XEXP (to, 0));
+      emit_move_insn (ptr, force_operand (XEXP (to, 0), NULL_RTX));
       to = replace_equiv_address (to, ptr);
-      emit_move_insn (rem, plus_constant (ptr_mode, rem, -blksize));
+      rtx rem_minus_blksize = plus_constant (ptr_mode, rem, -blksize);
+      emit_move_insn (rem, force_operand (rem_minus_blksize, NULL_RTX));
     }
 
   /* Iterate over power-of-two block sizes from the maximum length to
@@ -6809,9 +6810,10 @@  try_store_by_multiple_pieces (rtx to, rtx len, unsigned int ctz_len,
       /* Adjust REM and PTR, unless this is the last iteration.  */
       if (i != sctz_len)
 	{
-	  emit_move_insn (ptr, XEXP (to, 0));
+	  emit_move_insn (ptr, force_operand (XEXP (to, 0), NULL_RTX));
 	  to = replace_equiv_address (to, ptr);
-	  emit_move_insn (rem, plus_constant (ptr_mode, rem, -blksize));
+	  rtx rem_minus_blksize = plus_constant (ptr_mode, rem, -blksize);
+	  emit_move_insn (rem, force_operand (rem_minus_blksize, NULL_RTX));
 	}
 
       if (label)