RE: [PATCH] Fix mips_expand_synci_loop

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

Richard Sandiford wrote:
> >>>>   This bug was detected on Debian on a 24kf Malta board, 
> when I was
> >>>> doing a JIT application that needs to flush cache for 
> some generated MIPS code.
> >>>>
> >>>>   I found that my patch may skip the last synci for the 
> end address (-1).
> >>>> Maybe we need to append an extra synci at the end.
> >>>>
> >>> This is kind of ugly, but will probably work.
> >>>
> >>> Would it be more elegant to mask the base address with 
> ~(hwr_synci_step 
> >>> - 1) before entering the loop?
> >> 
> >> Yeah, I think that'd be better.  It might also be worth 
> using a separate
> >> length, so that the delay slot can be filled.  I.e. something like:
> >> 
> >
> > You also need:
> >
> > 	beq	zero, inc, skip_the_whole_thing.
> 
> And if begin == end is allowed, also:
> 
>         beq     begin, end, skip_the_whole_thing
> 
> Three bugs for the price of one.
> 

  Could we ignore these two tests: (inc == 0) and (begin == end)?
The original implementation doesn't have them, and
the performance may be affected.

  The patch to implement Richard's sequence is attached.
I used the GCC mainline with -msynci to compile WebKit JavaScriptCore
with MIPS JIT enabled.  JavaScriptCore tests are ok.

For GCC mips clear_cache* tests:

PASS: gcc.target/mips/clear-cache-1.c (test for excess errors)
PASS: gcc.target/mips/clear-cache-1.c scan-assembler synci
PASS: gcc.target/mips/clear-cache-1.c scan-assembler jr.hb
PASS: gcc.target/mips/clear-cache-1.c scan-assembler-not _flush_cache

PASS: gcc.target/mips/clear-cache-2.c (test for excess errors)
PASS: gcc.target/mips/clear-cache-2.c scan-assembler-not synci
PASS: gcc.target/mips/clear-cache-2.c scan-assembler-not jr.hb
PASS: gcc.target/mips/clear-cache-2.c scan-assembler _flush_cache

  Any comment?  Thanks a lot!

Regards,
Chao-ying

2009-10-27  Chao-ying Fu  <fu@mips.com>

	* config/mips/mips.c (mips_expand_synci_loop):  Used the length rtx
	to control the synci loop from the begin rtx that is pointed to the
	first byte of the cache line.

Index: mips.c
===================================================================
--- mips.c	(revision 153570)
+++ mips.c	(working copy)
@@ -6689,7 +6689,7 @@
 void
 mips_expand_synci_loop (rtx begin, rtx end)
 {
-  rtx inc, label, cmp, cmp_result;
+  rtx inc, label, cmp_result, mask, length;
 
   /* Load INC with the cache line size (rdhwr INC,$1).  */
   inc = gen_reg_rtx (Pmode);
@@ -6697,17 +6697,31 @@
 	     ? gen_rdhwr_synci_step_si (inc)
 	     : gen_rdhwr_synci_step_di (inc));
 
+  /* Calculate mask.  */
+  mask = gen_reg_rtx (Pmode);
+  mips_emit_binary (MINUS, mask, gen_rtx_REG (Pmode, GP_REG_FIRST), inc);
+
+  /* Mask out begin by mask.  */
+  mips_emit_binary (AND, begin, begin, mask);
+
+  /* Calculate length.  */
+  length = gen_reg_rtx (Pmode);
+  mips_emit_binary (MINUS, length, end, begin);
+
   /* Loop back to here.  */
   label = gen_label_rtx ();
   emit_label (label);
 
   emit_insn (gen_synci (begin));
 
-  cmp = mips_force_binary (Pmode, GTU, begin, end);
+  /* Update length.  */
+  mips_emit_binary (MINUS, length, length, inc);
 
+  /* Update begin.  */
   mips_emit_binary (PLUS, begin, begin, inc);
 
-  cmp_result = gen_rtx_EQ (VOIDmode, cmp, const0_rtx);
+  /* Check if length is greater than 0.  */
+  cmp_result = gen_rtx_GT (VOIDmode, length, const0_rtx);
   emit_jump_insn (gen_condjump (cmp_result, label));
 }