Re: [PATCH RFA] mips "Branch Likely" handling behaviour,options.

[cgd at broadcom dot com]

Wow, actual review!  thanks!  8-)

At Fri, 02 Aug 2002 05:55:30 -0700, Mark D. Baushke wrote:
> Chris wrote:
> >+	 use" is strongly encouraged).  If the use wants to enable them or
> 
> 'If the use wants' should be 'If the user wants'

indeed.


> I'll note that a command like:
> 
>   gcc ... -mno-branch-likely ... -mbranch-likely ...
> 
> would intuitively seem to want to become -mbranch-likely rather than
> generating a warning and using the no-branch-likely setting... That is,
> the last option should win unless it is not a supported target.

Yes.  This is annoying.  However, it's the best one can do (AFAICT)
using TARGET_OPTIONS since they just stuff strings into a variable per
option, leaving no clue as to the actual ordering.

The theory was do something fail-safe (don't generate) and warn that
the compiler might be doing something unexpected.

(Can't really use TARGET_SWITCHES for this either, since you need to
set one bit to indicate "switch was provided" and another to indicate
the setting...  which means setting and clearing a bit in the same go
which i don't think it can do.)


(Note that there are similar problems re: -mipsN and -mips16.  See the
discussion about configury support for --with-arch et al.)


> I suspect you will also need a warning to catch someone using
> both -mips1 -mbranch-likely on the same command-line. 

I considered that, and would be willing to implement it if Eric thinks
it appropriate.

My rational for not including it, btw, was the comment (now deleted):

-/* Disable branchlikely for tx39 until compare rewrite.  They haven't
-   been generated up to this point.  */

which made me think that somebody might want to enable it for tx39
(MIPS I) w/o enabling it by default.

I guess a warning wouldn't cause much harm...


> Maybe the line
> 
> >+#define GENERATE_BRANCHLIKELY   (!TARGET_MIPS16 && mips_use_branchlikely)
> 
> should be
> 
> #define GENERATE_BRANCHLIKELY   (!ISA_MIPS1 && !TARGET_MIPS16 \
> 				&& mips_use_branchlikely)
> 
> to avoid this case?

That wouldn't be the right thing; it would preclude you from ever (w/o
touching that code 8-) adding support for a MIPS I cpu that generates
branch-likely instructions.  Based on the r3900 comment, i believe
that such a thing exists.


What i'd do is:

* don't nuke ISA_HAS_BRANCHLIKELY (that could include 3900 later).

* use it to help set mips_use_branchlikely.

* if mips_use_branchlikely && !ISA_HAS_BRANCHLIKELY, warn.




cgd