Re: Patch: mips gas reorg

[cgd at sibyte dot com]

echristo@redhat.com ("Eric Christopher") writes:
> Here's a patch for the -march/mtune support. I extended HJ's original
> patch quite a bit and fixed some things therein.  Fixes a few of the
> tests.

Very nice.  Definitely a step in the right direction for the MIPS
code.

I've got a couple of questions, of course.  8-)


> +/* The argument of the -march= flag.  The architecture we are assembling.  */
> +static int mips_arch = CPU_UNKNOWN;
> +
> +/* The argument of the -mtune= flag.  The architecture for which we
> +   are optimizing.  */
> +static int mips_tune = CPU_UNKNOWN;

really, you mean "architecture or cpu" for both of these, right?


> -#define hilo_interlocks (mips_cpu == CPU_R4010                       \
> +#define hilo_interlocks (mips_tune == CPU_R4010                       \
>                           )
>  
>  /* Whether the processor uses hardware interlocks to protect reads
>     from the GPRs, and thus does not require nops to be inserted.  */
>  #define gpr_interlocks \
>    (mips_opts.isa != ISA_MIPS1  \
> -   || mips_cpu == CPU_R3900)
> +   || mips_tune == CPU_R3900)
>  
>  /* As with other "interlocks" this is used by hardware that has FP
>     (co-processor) interlocks.  */
>  /* Itbl support may require additional care here.  */
> -#define cop_interlocks (mips_cpu == CPU_R4300                        \
> +#define cop_interlocks (mips_tune == CPU_R4300                        \
>  			)

I'm puzzled by these.  in particular, if you've said -march=foo
-mtune=bar, that means "compile code that will run on properly foo,
i.e. uses only the architectural features of foo, but has had
scheduling optimization done for bar", right?

so, for instance, if you have:

	-march=mips1 -mtune=r3900

that means "generic mips1-compatible code, but tuned for the r3900".

However, e.g. the gpr_interlocks definition above would seem to
generate code that _will not_ run on a generic mips1 cpu if you say
-mtune=r3900 -- the assembler will emit instructions assuming that the
GPRs are adequately interlocked (i.e., it won't emit nops in certain
places), but that is _not_ necessarily true for mips1 CPUs.

I believe that what you need for these cases are conditionals like:

#define	have_gpr_interlocks (isa != MIPS1 || mips_arch == R3900)
#define	prefer_gpr_interlocks (1)
			/* OK, this case is simple -- is there ever a
			   case where you'd have a feature to use but
			   prefer to avoid it? */

then when using:

	if (have_gpr_interlocks && prefer_gpr_interlocks) {

or similar.  If there's no such case when you'd like to schedule to
avoid them, well, you can go back to using only one.


Am I missing something here?

(yes, that means that unless there's some kind of instruction
scheduling in gas, -mtune=* doesn't have much use in the
assembler... but that doesn't seem surprising to me...)



chris