Re: [Revised patch] Rework MIPS command-line handling

[cgd at broadcom dot com]

> More details:
> 
> 1) Internally, gcc treats -mips1 and -march=mips1 like -march=r2000,
>    and similarly for the other ISA levels.
> 
>    Question is, what are right values for _MIPS_ARCH and _MIPS_TUNE when
>    you use a generic ISA?  It doesn't seem right for them to name a
>    processor when the command line and configuration don't specify one.
>    So in the patch I've just set them to 0.  It gives users a way of
>    distinguishing between -march=mipsN and -march=<processor>, should
>    they want to.

I would advise simply going with whatever the processor is, unless
you're going to create 'real' entries for the arches
(e.g. _PROCESSOR_MIPS1.)


> 2) Each processor has a macro of the form _MIPS_<name>.  As suggested,
>    each known processor has distinct macro, even if GCC treats them
>    the same way internally.  So there's both a _MIPS_R2000 and
>    a _MIPS_R3000, for example.
> 
>    Not the biggest issue, I know, but is _MIPS_<foo> OK for the
>    processor macros?  I guess other alternatives would be
>    _MIPS_PROCESSOR_<foo> or _PROCESSOR_<foo>.

Personlly, I don't like having MIPS in the name.  Many processors are
not, and have never been, made by "MIPS" (MTI).  There's something to
be said that they all (should be) MIPS architecture licensees, but,
well, if somebody other than a licensee wants to ue the back-end and
is close enough, it would be broken to force them to have MIPS in
their name.

I'm for _PROCESSOR_*.


>    I guess the values of these macros ought not to change between
>    releases, just in case someone stores _MIPS_ARCH in a variable
>    in one compilation unit and reads it in another, compiled with
>    a different version of gcc.  So I've used a (lame!) convention
>    for giving each entry a unique value.  See comment above
>    mips_cpu_info_table[] in mips.c for details.

I don't see why the values shouldn't change.

(1) The numbers aren't really meaningful.

(2) if used properly (as outlined in my message which spawned this
    particular feature), they'll all be resolved at compile time.

If there's a desire to record the values, e.g. for diagnostic output
from programs, that should probably be done using defines that use
strings.


>    This should also make it easier to check for a range of
>    processors, such as members of a family that have different
>    tuning characteristics but the same basic ISA.

Only if those values become well known.

from where I sit:

(1) it's unreasonable to have people code numeric constants in their
    code or header files, for these values provided internally by GCC,
    since GCC _may_ have to change some of them in the future.

(2) it's unreasonable to pass all of these all the time on the command
    line.

(3) it's not really reasonable to have GCC provide a special header
    which defines all of them.

(4) without having them all around all the time, it's hard to use them
    for e.g. "range checking."


I've outlined (in off-list mail) a mechanism that would allow code to
check for specific CPUs being used in arch/tune which doesn't involve
any of this persistent, ad hoc numbering crud.  I think it's the right
way.

For the benefit of everybody else:

	assume the processors _PROCESSOR_FOO and _PROCESSOR_BAR are
	used by _MIPS_ARCH and _MIPS_TUNE

	define _PROCESSOR_FOO the value 1.

	iff FOO != BAR, define _PROCESSOR_BAR the value 2.

Since CPP math treats undefined values as 0, code like:

#if (_MIPS_ARCH == _PROCESSOR_LALALA)

will work properly, and code written as the example in my orignal
message:

	switch (_MIPS_ARCH) {
#ifdef _PROCESSOR_LALALA
	case _PROCESSOR_LALALA:
		...
		break;
#endif
	default:
		...
		break;
	}

will also work properly.

I believe that those are good examples of the style with which these
new defines should be used.


> 5) I prefer '-march=from-abi' over '-mstrict-abi' and '-mdeduce-arch'
>    (which were the other suggestions I saw).  For one thing, it brings
>    all architecture-changing stuff under -march, but more importantly,
>    a configuration can then have "from-abi" as its default architecture.

This does seem to be The Right Thing, at least to me.  8-)



> 6) The consensus seemed to be that, when using a configuration associated
>    with a particular ISA or processor, we should try to stick to the
>    default where possible.  So "mips64orion-elf-gcc -mabi=32" would
>    generate 32-bit 4600 code rather than than MIPS I code.
> 
>    Then, we would only deviate from the default architecture when that
>    architecture is a 32-bit one, and the selected ABI requires 64-bit
>    registers.  For example, "mipstx39-elf -mabi=o64".  For configs
>    like mipstx39-elf that are associated with a particular ISA, I
>    think specifying an incompatible ABI should be a hard error.
>    The user can of course say "mipstx39-elf -mabi=o64 -mips3" if
>    that's what they truly mean.
> 
>    The question is, how do people see configs like mips-elf and
>    mips64-elf?  [ ... ]

I think I agree with you here...



I don't have time to read all of the diffs right now, or to read them
deeply, but there are a couple of comments below.


chris
=======
>   # Other random stuff.
>   
> +     case ${cpu_type} in
> +       mips)
> + 	# Set mips_cpu to the name of the default CPU.
> + 	case ${target_cpu} in
> + 	  mips | mipsbe | mipseb | mipsle | mipsel | mips64 | mips64el)
> + 	    mips_cpu=from-abi
> + 	    ;;
> + 	  mipsisa32 | mipsisa32el)
> + 	    mips_cpu=mips32
> + 	    ;;
> + 	  mipsisa64 | mipsisa64el)
> + 	    mips_cpu=mips64
> + 	    ;;
> + 	  mipstx39 | mipstx39el)
> + 	    mips_cpu=r3900
> + 	    ;;
> + 	  mips64* | mipsisa64*)
> + changequote(,)dnl
> + 	    mips_cpu=`echo $target_cpu | sed -e 's/[a-z]*64//' -e 's/el$//'`
> + changequote([,])dnl
> + 	    ;;

Why not do the same here for mipsisa32*?  (OK, nothing uses it yet...
8-)

> + 	# Decide whether to generate 32-bit or 64-bit code by default.
> + 	# Used to resolve -march=from-abi when an embedded ABI is selected.
> + 	case ${target_cpu} in
> + 	  mips64* | mipsisa64*)
> + 	    mips_default_64bit=1
> + 	    ;;
> + 	  *)
> + 	    mips_default_64bit=0
> + 	    ;;

Might be desirable to provide an easy, existing way/place to override
this based on the OS in question.