head: MIPS: Workaround an R4000SC/MC branch/divide erratum

Richard Sandiford rsandifo@redhat.com
Mon Mar 1 18:35:00 GMT 2004


"Maciej W. Rozycki" <macro@ds2.pg.gda.pl> writes:
>  I'm not particularly happy about that sprintf() that does not prevent 
> buffer overflows, but that's what's used elsewhere.  The string passed to 
> mips_output_division() is currently fixed in length, so no problem will 
> appear unless that changes.

Yeah, I'd really rather avoid a static buffer if possible.  I know
it's used in mips_output_conditional_branch(), but that's not really
a good example to follow.

> @@ -9121,20 +9174,27 @@ const char *
>  mips_output_division (const char *division, rtx *operands)
>  {
>    const char *s = division;
> +  static char buffer[30];
>  
>    if (TARGET_CHECK_ZERO_DIV)
>      {
> -      output_asm_insn (s, operands);
> +
> +      if (TARGET_FIX_R4000 || TARGET_FIX_R4400)
> +	sprintf (buffer, "%%(%s%%)", division);
> +      else
> +	sprintf (buffer, "%s", division);
> +      output_asm_insn (buffer, operands);
>  
>        if (TARGET_MIPS16)
>  	s = "bnez\t%2,1f\n\tbreak\t7\n1:";
>        else
>  	s = "bne\t%2,%.,1f%#\n\tbreak\t7\n1:";
>      }
> -  if (TARGET_FIX_R4000 || TARGET_FIX_R4400)
> +  else if (TARGET_FIX_R4000 || TARGET_FIX_R4400)
>      {
> -      output_asm_insn (s, operands);
> -      s = "nop";
> +      sprintf (buffer, "%%(%s", division);
> +      output_asm_insn (buffer, operands);
> +      s = "nop%)";
>      }
>    return s;
>  }

I'm not too keen on the "sprintf (..., "%s", ...)" bit.  Better to just
call output_asm_insn in the TARGET_FIX* arm.

Anyhow, after the change to force division into the delay slot,
I think all we need to do is swap the clauses around, like so:

*************** mips_output_division (const char *divisi
*** 9223,9228 ****
--- 9223,9233 ----
    const char *s;
  
    s = division;
+   if (TARGET_FIX_R4000 || TARGET_FIX_R4400)
+     {
+       output_asm_insn (s, operands);
+       s = "nop";
+     }
    if (TARGET_CHECK_ZERO_DIV)
      {
        if (TARGET_MIPS16)
*************** mips_output_division (const char *divisi
*** 9236,9246 ****
  	  output_asm_insn (s, operands);
  	  s = "break\t7%)\n1:";
  	}
-     }
-   if (TARGET_FIX_R4000)
-     {
-       output_asm_insn (s, operands);
-       s = "nop";
      }
    return s;
  }
--- 9241,9246 ----

If you agree, I'll test your patch with that change.

Richard



More information about the Gcc-patches mailing list