Re: [PATCH] arm: Fix ICE with CMSE nonsecure call on Armv8.1-M [PR100333]

[Richard Earnshaw via Gcc-patches]

On 30/04/2021 09:30, Alex Coplan via Gcc-patches wrote:
Hi,

As the PR shows, we ICE shortly after expanding nonsecure calls for
Armv8.1-M.  For Armv8.1-M, we have TARGET_HAVE_FPCXT_CMSE. As it stands,
the expander (arm.md:nonsecure_call_internal) moves the callee's address
to a register (with copy_to_suggested_reg) only if
!TARGET_HAVE_FPCXT_CMSE.

However, looking at the pattern which the insn appears to be intended to
match (thumb2.md:*nonsecure_call_reg_thumb2_fpcxt), it requires the
callee's address to be in a register.

This patch therefore just forces the callee's address into a register in
the expander.

Testing:
  * Regtested an arm-eabi cross configured with
  --with-arch=armv8.1-m.main+mve.fp+fp.dp --with-float=hard. No regressions.
  * Bootstrap and regtest on arm-linux-gnueabihf in progress.

OK for trunk and backports as appropriate if bootstrap looks good?

Thanks,
Alex

gcc/ChangeLog:

        PR target/100333
        * config/arm/arm.md (nonsecure_call_internal): Always ensure
        callee's address is in a register.

gcc/testsuite/ChangeLog:

        PR target/100333
        * gcc.target/arm/cmse/pr100333.c: New test.



-  "
   {
-    if (!TARGET_HAVE_FPCXT_CMSE)
-      {
-       rtx tmp =
-         copy_to_suggested_reg (XEXP (operands[0], 0),
-                                gen_rtx_REG (SImode, R4_REGNUM),
-                                SImode);
+    rtx tmp = NULL_RTX;
+    rtx addr = XEXP (operands[0], 0);

-       operands[0] = replace_equiv_address (operands[0], tmp);
-      }
-  }")
+    if (TARGET_HAVE_FPCXT_CMSE && !REG_P (addr))
+      tmp = force_reg (SImode, addr);
+    else if (!TARGET_HAVE_FPCXT_CMSE)
+      tmp = copy_to_suggested_reg (XEXP (operands[0], 0),
+                                  gen_rtx_REG (SImode, R4_REGNUM),
+                                  SImode);


I think it might be better to handle the !TARGET_HAVE_FPCXT_CMSE case 
via a pseudo as well, then we don't end up generating a potentially 
non-trivial insn that directly writes a fixed hard reg - it's better to 
let later passes clean that up if they can.

Also, you've extracted XEXP (operands[0], 0) into 'addr', but then 
continue to use the XEXP form in the existing path.  Please be 
consistent use XEXP directly everywhere, or use 'addr' everywhere.

So you want something like

  addr = XEXP (operands[0], 0);
  if (!REG_P (addr))
    addr = force_reg (SImode, addr);

  if (!T_H_F_C)
    addr = copy...(addr, gen(r4), SImode);

  operands[0] = replace_equiv_addr (operands[0], addr);

R.

R.