[Patch] MIPS: Fix PR33479 Broken atomic memory primitives.

Thiemo Seufer ths@networkno.de
Sun Sep 23 16:22:00 GMT 2007


Richard Sandiford wrote:
> David Daney <ddaney@avtrex.com> writes:
> > 2007-09-22  David Daney  <ddaney@avtrex.com>
> >
> >     PR target/33479
> >     * config/mips/mips.md (sync_compare_and_swap<mode>, sync_old_add<mode>,
> >     sync_new_add<mode>, sync_old_<optab><mode>, sync_new_<optab><mode>,
> >     sync_old_nand<mode>, sync_new_nand<mode>,
> >     sync_lock_test_and_set<mode>): Fix '&' constraint modifiers.  Set
> >     mips_branch_likely.  Update length attributes.
> >     (sync_add<mode>, sync_sub<mode>, sync_old_sub<mode>,
> >     sync_new_sub<mode>, sync_<optab><mode>, sync_nand<mode>): Set
> >     mips_branch_likely.  Update length attributes.
> >     * config/mips/mips.h (GENERATE_BRANCHLIKELY_FOR_ATOMIC_OP): Define new
> >     predicate.
> >     (MIPS_COMPARE_AND_SWAP, MIPS_SYNC_OP, MIPS_SYNC_OLD_OP,
> >     MIPS_SYNC_NEW_OP, MIPS_SYNC_NAND, MIPS_SYNC_OLD_NAND,
> >     MIPS_SYNC_NEW_NAND, MIPS_SYNC_EXCHANGE): Rewrite.
> >     * doc/invoke.texi (-mbranch-likely): Document option interaction with
> >     atomic memory operations.
> 
> This is fine if it's the way we decide to go.  The default branch-likely
> handling is becoming a bit of a dog's dinner, but I don't know how that
> can be helped.  One of the suggestions in the gcc@ thread was to use
> -mfix-*, but the fix options are supposed to work within the constraints
> of the selected ISA, so it wouldn't solve the problem of whether
> branch-likely instructions should be considered available in the
> first place.
> 
> I know Ralf said the instructions were only deprecated on paper,
> but presumably no known MIPS32Rx or MIPS64Rx processor requires
> branch likely, so it would still be possible to disable them
> when such an ISA is selected.  I.e., we could delete:
> 
>   if ((target_flags_explicit & MASK_BRANCHLIKELY) == 0)
>     {
>       /* If neither -mbranch-likely nor -mno-branch-likely was given
> 	 on the command line, set MASK_BRANCHLIKELY based on the target
> 	 architecture.
> 
> 	 By default, we enable use of Branch Likely instructions on
> 	 all architectures which support them with the following
> 	 exceptions: when creating MIPS32 or MIPS64 code, and when
> 	 tuning for architectures where their use tends to hurt
> 	 performance.
> 
> 	 The MIPS32 and MIPS64 architecture specifications say "Software
> 	 is strongly encouraged to avoid use of Branch Likely
> 	 instructions, as they will be removed from a future revision
> 	 of the [MIPS32 and MIPS64] architecture."  Therefore, we do not
> 	 issue those instructions unless instructed to do so by
> 	 -mbranch-likely.  */
>       if (ISA_HAS_BRANCHLIKELY
> 	  && !(ISA_MIPS32 || ISA_MIPS32R2 || ISA_MIPS64)
> 	  && !(TUNE_MIPS5500 || TUNE_SB1))
> 	target_flags |= MASK_BRANCHLIKELY;
>       else
> 	target_flags &= ~MASK_BRANCHLIKELY;

AFAICS this is broken. A -mtune option should never influence
correctness of the code. E.g. "-march=mips4 -mtune=r5500" won't
work on R10000.


Thiemo



More information about the Gcc-patches mailing list