Re: [PATCH] New MIPS interrupt handler patch

["Fu, Chao-Ying" <fu at mips dot com>]

"Richard Sandiford" wrote:
> "Fu, Chao-Ying" <fu@mips.com> writes:
> >> > >   One remaining issue is that the assembler complains about
> >> > > "used $at without ".set noat", because we access $1.
> >> > > How do we avoid it?
> >> >
> >> > Oh yeah, I was going to ask about that.
> >> >
> >> > We could either do something in FINAL_PRESCAN_INSN (and add a new
> >> > FINAL_POSTSCAN_INSN counterpart) or do something special in
> >> > mips_output_move.  I kind-of prefer the first.
> >> >
> >> > Let's handle this separately from the main patch though.
> >>
> >>   Yes.
> >>
> >
> > Hi,
> >
> >   This patch handles the issue of ".set noat" and ".set at" for
> > MIPS interrupt handlers by using FINAL_PRESCAN_INSN and
> > FIANL_POSTSCAN_INSN.
>
> Thanks for following up with this.
>
> >   Is the patch ok?  Thanks!
>
> I can't approve the final.c bits, but as far as MIPS goes...
>
> > +/* Implement FINAL_PRESCAN_INSN.  */
> > +
> > +void
> > +mips_final_prescan_insn (rtx insn ATTRIBUTE_UNUSED, rtx *opvec, int
noperands)
> > +{
> > +  int i;
> > +  for (i = 0; i < noperands; i++)
> > +    {
> > +      rtx op = opvec[i];
> > +      if (REG_P (op) && REGNO (op) == AT_REGNUM)
> > + {
> > +   fprintf (asm_out_file, "\t.set\tnoat\n");
> > +   return;
> > + }
> > +    }
> > +}
>
> I think you should use for_each_rtx on the operands instead, so that we
> handle things like $1 addresses.  You should also skip asms; it's up to
> them to emit ".set noat" and ".set at" where appropriate.  A simple test
> for this is:
>
>     if (recog_memoized (insn) >= 0)
>
> (Remember to remove the ATTRIBUTE_UNUSED if you do this.)
>
> (For the record, I agree we shouldn't look at the instruction pattern,
> just the operands.  If something ever does have $1 hard-coded into its
> pattern, it should handle the at/noat stuff itself.)
>
> The fprintf should be guarded with:
>
>       if (set_noat++ == 0)
>
> just like the %[ code is.  The corresponding final_postscan_insn
> code should have:
>
>       if (--set_noat == 0)
>
> Please add a testcase too.  An interrupt handler test marked with:
>
>   /* { dg-do assemble } */
>
> should be fine.
>

  Yes.  Here is the updated version based on your review.  Thanks a lot!

Regards,
Chao-ying

Index: final.c
===================================================================
--- final.c (revision 145780)
+++ final.c (working copy)
@@ -2235,6 +2235,10 @@ final_scan_insn (rtx insn, FILE *file, i
 #endif
        }

+#ifdef FINAL_POSTSCAN_INSN
+     FINAL_POSTSCAN_INSN (insn, ops, insn_noperands);
+#endif
+
      this_is_asm_operands = 0;
      break;
    }
@@ -2637,6 +2641,13 @@ final_scan_insn (rtx insn, FILE *file, i
  /* Output assembler code from the template.  */
  output_asm_insn (templ, recog_data.operand);

+ /* Some target machines need to postscan each insn after
+    it is output.  */
+
+#ifdef FINAL_POSTSCAN_INSN
+ FINAL_POSTSCAN_INSN (insn, recog_data.operand, recog_data.n_operands);
+#endif
+
  /* If necessary, report the effect that the instruction has on
     the unwind info.   We've already done this for delay slots
     and call instructions.  */
Index: doc/tm.texi
===================================================================
--- doc/tm.texi (revision 145780)
+++ doc/tm.texi (working copy)
@@ -8193,6 +8193,12 @@ writing conditional output routines in t
 If this macro is not defined, it is equivalent to a null statement.
 @end defmac

+@defmac FINAL_POSTSCAN_INSN (@var{insn}, @var{opvec}, @var{noperands})
+If defined, a C statement to be executed just after the output of
+assembler code for @var{insn}.  This macro is similar to
+@code{FINAL_PRESCAN_INSN}.
+@end defmac
+
 @defmac PRINT_OPERAND (@var{stream}, @var{x}, @var{code})
 A C compound statement to output to stdio stream @var{stream} the
 assembler syntax for an instruction operand @var{x}.  @var{x} is an
Index: config/mips/mips.h
===================================================================
--- config/mips/mips.h (revision 145780)
+++ config/mips/mips.h (working copy)
@@ -3459,3 +3459,10 @@ extern enum mips_code_readable_setting m

 /* Enable querying of DFA units.  */
 #define CPU_UNITS_QUERY 1
+
+/* For an instruction that accesses $1 (AT), we need to output ".set noat"
+   before the instruction, and output ".set at" after the instruction.  */
+#define FINAL_PRESCAN_INSN(INSN, OPVEC, NOPERANDS) \
+  mips_final_prescan_insn (INSN, OPVEC, NOPERANDS);
+#define FINAL_POSTSCAN_INSN(INSN, OPVEC, NOPERANDS) \
+  mips_final_postscan_insn (INSN, OPVEC, NOPERANDS);
Index: config/mips/mips-protos.h
===================================================================
--- config/mips/mips-protos.h (revision 145780)
+++ config/mips/mips-protos.h (working copy)
@@ -333,5 +333,7 @@ extern void mips_expand_atomic_qihi (uni
 extern void mips_expand_vector_init (rtx, rtx);

 extern bool mips_epilogue_uses (unsigned int);
+extern void mips_final_prescan_insn (rtx, rtx *, int);
+extern void mips_final_postscan_insn (rtx, rtx *, int);

 #endif /* ! GCC_MIPS_PROTOS_H */
Index: config/mips/mips.c
===================================================================
--- config/mips/mips.c (revision 145780)
+++ config/mips/mips.c (working copy)
@@ -14697,6 +14697,41 @@ mips_epilogue_uses (unsigned int regno)

   return false;
 }
+
+/* A for_each_rtx callback.  Stop the search if *X is an AT register.  */
+
+static int
+mips_at_reg_p (rtx *x, void *data ATTRIBUTE_UNUSED)
+{
+  return GET_CODE (*x) == REG && REGNO (*x) == AT_REGNUM;
+}
+
+
+/* Implement FINAL_PRESCAN_INSN.  */
+
+void
+mips_final_prescan_insn (rtx insn, rtx *opvec, int noperands)
+{
+  int i;
+  if (recog_memoized (insn) >= 0)
+    for (i = 0; i < noperands; i++)
+      if (for_each_rtx (&opvec[i], mips_at_reg_p, NULL))
+ if (set_noat++ == 0)
+   fprintf (asm_out_file, "\t.set\tnoat\n");
+}
+
+/* Implement FINAL_POSTSCAN_INSN.  */
+
+void
+mips_final_postscan_insn (rtx insn, rtx *opvec, int noperands)
+{
+  int i;
+  if (recog_memoized (insn) >= 0)
+    for (i = 0; i < noperands; i++)
+      if (for_each_rtx (&opvec[i], mips_at_reg_p, NULL))
+ if (--set_noat == 0)
+   fprintf (asm_out_file, "\t.set\tat\n");
+}
 
 /* Initialize the GCC target structure.  */
 #undef TARGET_ASM_ALIGNED_HI_OP
Index: testsuite/gcc.target/mips/interrupt_handler.c
===================================================================
--- testsuite/gcc.target/mips/interrupt_handler.c (revision 145780)
+++ testsuite/gcc.target/mips/interrupt_handler.c (working copy)
@@ -1,5 +1,5 @@
 /* Test attributes for interrupt handlers */
-/* { dg-do compile } */
+/* { dg-do assemble } */
 /* { dg-options "-mips32r2 -msoft-float" } */

 void f () { }