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

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

[Andrew Stubbs]

[-- Attachment #1: Type: text/plain, Size: 552 bytes --]

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.

[-- Attachment #2: 210507-fix-try-store.patch --]
[-- Type: text/plain, Size: 1635 bytes --]

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.

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)

Re: [PATCH] builtins.c: Ensure emit_move_insn operands are valid (PR100418)

[Jeff Law]

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