public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* PATCH:  mips16/nomips16 function attributes, version N
@ 2007-08-27 20:14 Sandra Loosemore
  2007-08-28  9:02 ` Richard Sandiford
  0 siblings, 1 reply; 5+ messages in thread
From: Sandra Loosemore @ 2007-08-27 20:14 UTC (permalink / raw)
  To: GCC Patches; +Cc: richard, David Ung, Nigel Stephens, Mark Mitchell

[-- Attachment #1: Type: text/plain, Size: 866 bytes --]

Here is the current version of the MIPS-specific part of the 
mips16/nomips16 function attributes patch.  This is identical to the 
last version I posted a couple of weeks ago, with two exceptions:

* The per-function hook has been renamed to reflect the new 
TARGET_SET_CURRENT_FUNCTION definition in the target-independent part of 
the patch (which I just posted separately).

* I've implemented Richard's suggestion of taking out the call to 
mips_set_mips16_mode in override_options, and just waiting until the 
per-function hook has been invoked for the first time.

-mflip-mips16 test results using the new hook function are pretty much 
identical to those from the last version, except that the one test case 
that was formerly giving an ICE because tree optimizations weren't 
seeing the same optabs as RTL generation no longer does so.  Yay!  :-)

-Sandra


[-- Attachment #2: 30a-mips16-funcattr.log --]
[-- Type: text/x-log, Size: 2889 bytes --]

2007-08-27  Sandra Loosemore  <sandra@codesourcery.com>
	    David Ung  <davidu@mips.com>
            Nigel Stephens <nigel@mips.com>

	gcc/
	Add mips16/nomips16 function attributes and -mflip-mips16 option
	for testing mixed-mode compilation.

	* config/mips/mips.opt (mflip-mips16): New.

	* config/mips/mips.h (SYMBOL_FLAG_MIPS16_FUNC): Define.
	(SYMBOL_FLAG_MIPS16_FUNC_P): Define.

	* config/mips/mips.c (mips_base_target_flags): New.
	(mips_base_mips16): New.
	(mips_base_schedule_insns): New.
	(mips_base_reorder_blocks_and_partition): New.
	(mips_base_align_loops): New.
	(mips_base_align_jumps): New.
	(mips_base_align_functions): New.
	(mips16_flipper): New.
	(mips_attribute_table): Add "mips16" and "nomips16" entries.
	(TARGET_SET_CURRENT_FUNCTION): Define.
	(mips_mips16_type_p, mips_nomips16_type_p): New.
	(mips_comp_type_attributes): Check mips16/nomips16 attributes.
	(mips_cannot_force_const_mem): Check current have_tls setting, not
	base setting for file.
	(mips_function_ok_for_sibcall): Make it deal with functions with
	mips16 attributes.
	(function_arg): Must check base mips16 mode for file, not just
	current setting.
	(mips_init_split_addresses): New, split out from override_options.
	(mips_init_relocs): New, split out from override_options.
	(was_mips16_p): New.
	(mips_set_mips16_mode): New, split out from override_options.
	(mips_set_current_function): New.
	(override_options):  Save base option settings.
	(mips_file_start): Move mips16 mode setting output from here....
	(mips_output_function_prologue): ....to here.
	(mips_output_mi_thunk): Check for mips16 function.
	(build_mips16_function_stub): Don't set .mips16 here.
	(build_mips16_call_stub): Likewise.
	(mips_expand_builtin): Error in mips16 mode.
	(mips_use_mips16_mode_p): New.
	(mips_encode_section_info): Check for mips16 function, and set
	SYMBOL_REF_FLAGS accordingly.

	* doc/extend.texi (Function Attributes): Document new
	mips16/nomips16 attributes.
	* doc/invoke.texi (Option Summary): Add -mflip-mips16.
	(MIPS Options): Document -mflip-mips16.

	* testsuite/gcc.target/mips/mips16-attributes.c: New.
	* testsuite/gcc.c-torture/compile/mipscop-1.c: Use nomips16
	attribute instead of using #ifndef to disable test for mips16.
	* testsuite/gcc.c-torture/compile/mipscop-2.c: Likewise.
	* testsuite/gcc.c-torture/compile/mipscop-3.c: Likewise.
	* testsuite/gcc.c-torture/compile/mipscop-4.c: Likewise.
	* testsuite/gcc.dg/torture/mips-hilo-1.c: Likewise.
	* testsuite/gcc.dg/torture/mips-hilo-2.c: Likewise.
	* testsuite/gcc.dg/torture/pr19683-1.c: Likewise.
	* testsuite/gcc.target/mips/madd-3.c: Add nomips16 attributes.
	* testsuite/gcc.target/mips/maddu-3.c: Likewise.
	* testsuite/gcc.target/mips/msub-3.c: Likewise.
	* testsuite/gcc.target/mips/msubu-3.c: Likewise.
	* testsuite/gcc.target/mips/dspr2-MULT.c: Likewise.
	* testsuite/gcc.target/mips/dspr2-MULTU.c: Likewise.
	



[-- Attachment #3: 30a-mips16-funcattr.patch --]
[-- Type: text/x-patch, Size: 43579 bytes --]

Index: gcc/config/mips/mips.opt
===================================================================
*** gcc/config/mips/mips.opt	(revision 127832)
--- gcc/config/mips/mips.opt	(working copy)
*************** mbranch-likely
*** 42,47 ****
--- 42,51 ----
  Target Report Mask(BRANCHLIKELY)
  Use Branch Likely instructions, overriding the architecture default
  
+ mflip-mips16
+ Target Report Var(TARGET_FLIP_MIPS16)
+ Switch on/off mips16 ASE on alternating functions for compiler testing
+ 
  mcheck-zero-division
  Target Report Mask(CHECK_ZERO_DIV)
  Trap on integer divide by zero
Index: gcc/config/mips/mips.h
===================================================================
*** gcc/config/mips/mips.h	(revision 127832)
--- gcc/config/mips/mips.h	(working copy)
*************** typedef struct mips_args {
*** 2294,2299 ****
--- 2294,2304 ----
  #define SYMBOL_REF_LONG_CALL_P(X)					\
    ((SYMBOL_REF_FLAGS (X) & SYMBOL_FLAG_LONG_CALL) != 0)
  
+ /* Flag to mark a function decl symbol a "mips16" function.  */
+ #define SYMBOL_FLAG_MIPS16_FUNC	(SYMBOL_FLAG_MACH_DEP << 1)
+ #define SYMBOL_REF_MIPS16_FUNC_P(RTX) \
+   ((SYMBOL_REF_FLAGS (RTX) & SYMBOL_FLAG_MIPS16_FUNC) != 0)
+ 
  /* True if we're generating a form of MIPS16 code in which jump tables
     are stored in the text section and encoded as 16-bit PC-relative
     offsets.  This is only possible when general text loads are allowed,
Index: gcc/config/mips/mips.c
===================================================================
*** gcc/config/mips/mips.c	(revision 127832)
--- gcc/config/mips/mips.c	(working copy)
*************** static rtx mips_expand_builtin_bposge (e
*** 424,429 ****
--- 424,431 ----
  static void mips_encode_section_info (tree, rtx, int);
  static void mips_extra_live_on_entry (bitmap);
  static int mips_comp_type_attributes (const_tree, const_tree);
+ static void mips_set_mips16_mode (int);
+ static void mips_set_current_function (tree);
  static int mips_mode_rep_extended (enum machine_mode, enum machine_mode);
  static bool mips_offset_within_alignment_p (rtx, HOST_WIDE_INT);
  static void mips_output_dwarf_dtprel (FILE *, int, rtx) ATTRIBUTE_UNUSED;
*************** int mips_abi = MIPS_ABI_DEFAULT;
*** 617,622 ****
--- 619,636 ----
  /* Cost information to use.  */
  const struct mips_rtx_cost_data *mips_cost;
  
+ /* Remember the ambient target flags, excluding mips16.  */
+ static GTY(()) int mips_base_target_flags;
+ /* The mips16 command-line target flags only.  */
+ static GTY(()) int mips_base_mips16;
+ /* Similar copies of option settings.  */
+ static int mips_base_schedule_insns; /* flag_schedule_insns */
+ static int mips_base_reorder_blocks_and_partition; /* flag_reorder... */
+ static int mips_base_align_loops; /* align_loops */
+ static int mips_base_align_jumps; /* align_jumps */
+ static int mips_base_align_functions; /* align_functions */
+ static GTY(()) int mips16_flipper;
+ 
  /* The -mtext-loads setting.  */
  enum mips_code_readable_setting mips_code_readable = CODE_READABLE_YES;
  
*************** const struct attribute_spec mips_attribu
*** 715,720 ****
--- 729,737 ----
    { "long_call",   0, 0, false, true,  true,  NULL },
    { "far",     	   0, 0, false, true,  true,  NULL },
    { "near",        0, 0, false, true,  true,  NULL },
+   /* Switch MIPS16 ASE on and off per-function.  */
+   { "mips16", 	   0, 0, false, true,  true,  NULL },
+   { "nomips16",    0, 0, false, true,  true,  NULL },
    { NULL,	   0, 0, false, false, false, NULL }
  };
  \f
*************** static const unsigned char mips16e_save_
*** 1251,1256 ****
--- 1268,1276 ----
  #undef TARGET_FUNCTION_OK_FOR_SIBCALL
  #define TARGET_FUNCTION_OK_FOR_SIBCALL mips_function_ok_for_sibcall
  
+ #undef TARGET_SET_CURRENT_FUNCTION
+ #define TARGET_SET_CURRENT_FUNCTION mips_set_current_function
+ 
  #undef TARGET_VALID_POINTER_MODE
  #define TARGET_VALID_POINTER_MODE mips_valid_pointer_mode
  #undef TARGET_RTX_COSTS
*************** mips_far_type_p (const_tree type)
*** 1369,1374 ****
--- 1389,1407 ----
  	  || lookup_attribute ("far", TYPE_ATTRIBUTES (type)) != NULL);
  }
  
+ /* Similar predicates for "mips16"/"nomips16" attributes.  */
+ 
+ static bool
+ mips_mips16_type_p (tree type)
+ {
+   return lookup_attribute ("mips16", TYPE_ATTRIBUTES (type)) != NULL;
+ }
+ 
+ static bool
+ mips_nomips16_type_p (tree type)
+ {
+   return lookup_attribute ("nomips16", TYPE_ATTRIBUTES (type)) != NULL;
+ }
  
  /* Return 0 if the attributes for two types are incompatible, 1 if they
     are compatible, and 2 if they are nearly compatible (which causes a
*************** mips_comp_type_attributes (const_tree ty
*** 1387,1392 ****
--- 1420,1430 ----
    if (mips_near_type_p (type1) && mips_far_type_p (type2))
      return 0;
  
+   /* Mips16/nomips16 attributes must match exactly.  */
+   if (mips_nomips16_type_p (type1) != mips_nomips16_type_p (type2)
+       || mips_mips16_type_p (type1) != mips_mips16_type_p (type2))
+     return 0;
+ 
    return 1;
  }
  \f
*************** mips_cannot_force_const_mem (rtx x)
*** 1777,1783 ****
  	return true;
      }
  
!   if (TARGET_HAVE_TLS && for_each_rtx (&x, &mips_tls_symbol_ref_1, 0))
      return true;
  
    return false;
--- 1815,1821 ----
  	return true;
      }
  
!   if (targetm.have_tls && for_each_rtx (&x, &mips_tls_symbol_ref_1, 0))
      return true;
  
    return false;
*************** mips_expand_call (rtx result, rtx addr, 
*** 3782,3794 ****
  }
  
  
! /* We can handle any sibcall when TARGET_SIBCALLS is true.  */
  
  static bool
! mips_function_ok_for_sibcall (tree decl ATTRIBUTE_UNUSED,
  			      tree exp ATTRIBUTE_UNUSED)
  {
!   return TARGET_SIBCALLS;
  }
  \f
  /* Emit code to move general operand SRC into condition-code
--- 3820,3843 ----
  }
  
  
! /* Implement TARGET_FUNCTION_OK_FOR_SIBCALL.  */
  
  static bool
! mips_function_ok_for_sibcall (tree decl,
  			      tree exp ATTRIBUTE_UNUSED)
  {
!   /* Not if TARGET_SIBCALLS isn't set.  */
!   if (!TARGET_SIBCALLS)
!     return 0;
! 
!   /* We can't do a sibcall if the called function is a MIPS16 function
!      because there is no direct "jx" instruction equivalent to "jalx" to
!      switch the ISA mode.  */
!   if (decl && SYMBOL_REF_MIPS16_FUNC_P (XEXP (DECL_RTL (decl), 0)))
!     return 0;
! 
!   /* Otherwise OK.  */
!   return 1;
  }
  \f
  /* Emit code to move general operand SRC into condition-code
*************** function_arg (const CUMULATIVE_ARGS *cum
*** 4225,4231 ****
       stored as the mode.  */
    if (mode == VOIDmode)
      {
!       if (TARGET_MIPS16 && cum->fp_code != 0)
  	return gen_rtx_REG ((enum machine_mode) cum->fp_code, 0);
  
        else
--- 4274,4281 ----
       stored as the mode.  */
    if (mode == VOIDmode)
      {
!       if ((TARGET_MIPS16 || mips_base_mips16)
! 	  && cum->fp_code != 0)
  	return gen_rtx_REG ((enum machine_mode) cum->fp_code, 0);
  
        else
*************** mips_set_tune (const struct mips_cpu_inf
*** 5031,5036 ****
--- 5081,5312 ----
      }
  }
  
+ /* mips_split_addresses is a half-way house between explicit
+    relocations and the traditional assembler macros.  It can
+    split absolute 32-bit symbolic constants into a high/lo_sum
+    pair but uses macros for other sorts of access.
+    
+    Like explicit relocation support for REL targets, it relies
+    on GNU extensions in the assembler and the linker.
+ 
+    Although this code should work for -O0, it has traditionally
+    been treated as an optimization.  */
+ 
+ static void
+ mips_init_split_addresses (void)
+ {
+   if (!TARGET_MIPS16 && TARGET_SPLIT_ADDRESSES
+       && optimize && !flag_pic
+       && !ABI_HAS_64BIT_SYMBOLS)
+     mips_split_addresses = 1;
+   else
+     mips_split_addresses = 0;
+ }
+ 
+ /* (Re-)Initialize information about relocs.  */
+ 
+ static void
+ mips_init_relocs (void)
+ {
+   memset (mips_split_p, '\0', sizeof (mips_split_p));
+   memset (mips_hi_relocs, '\0', sizeof (mips_hi_relocs));
+   memset (mips_lo_relocs, '\0', sizeof (mips_lo_relocs));
+ 
+   if (ABI_HAS_64BIT_SYMBOLS)
+     {
+       if (TARGET_EXPLICIT_RELOCS)
+ 	{
+ 	  mips_split_p[SYMBOL_64_HIGH] = true;
+ 	  mips_hi_relocs[SYMBOL_64_HIGH] = "%highest(";
+ 	  mips_lo_relocs[SYMBOL_64_HIGH] = "%higher(";
+ 
+ 	  mips_split_p[SYMBOL_64_MID] = true;
+ 	  mips_hi_relocs[SYMBOL_64_MID] = "%higher(";
+ 	  mips_lo_relocs[SYMBOL_64_MID] = "%hi(";
+ 
+ 	  mips_split_p[SYMBOL_64_LOW] = true;
+ 	  mips_hi_relocs[SYMBOL_64_LOW] = "%hi(";
+ 	  mips_lo_relocs[SYMBOL_64_LOW] = "%lo(";
+ 
+ 	  mips_split_p[SYMBOL_ABSOLUTE] = true;
+ 	  mips_lo_relocs[SYMBOL_ABSOLUTE] = "%lo(";
+ 	}
+     }
+   else
+     {
+       if (TARGET_EXPLICIT_RELOCS || mips_split_addresses || TARGET_MIPS16)
+ 	{
+ 	  mips_split_p[SYMBOL_ABSOLUTE] = true;
+ 	  mips_hi_relocs[SYMBOL_ABSOLUTE] = "%hi(";
+ 	  mips_lo_relocs[SYMBOL_ABSOLUTE] = "%lo(";
+ 
+ 	  mips_lo_relocs[SYMBOL_32_HIGH] = "%hi(";
+ 	}
+     }
+ 
+   if (TARGET_MIPS16)
+     {
+       /* The high part is provided by a pseudo copy of $gp.  */
+       mips_split_p[SYMBOL_GP_RELATIVE] = true;
+       mips_lo_relocs[SYMBOL_GP_RELATIVE] = "%gprel(";
+     }
+ 
+   if (TARGET_EXPLICIT_RELOCS)
+     {
+       /* Small data constants are kept whole until after reload,
+ 	 then lowered by mips_rewrite_small_data.  */
+       mips_lo_relocs[SYMBOL_GP_RELATIVE] = "%gp_rel(";
+ 
+       mips_split_p[SYMBOL_GOT_PAGE_OFST] = true;
+       if (TARGET_NEWABI)
+ 	{
+ 	  mips_lo_relocs[SYMBOL_GOTOFF_PAGE] = "%got_page(";
+ 	  mips_lo_relocs[SYMBOL_GOT_PAGE_OFST] = "%got_ofst(";
+ 	}
+       else
+ 	{
+ 	  mips_lo_relocs[SYMBOL_GOTOFF_PAGE] = "%got(";
+ 	  mips_lo_relocs[SYMBOL_GOT_PAGE_OFST] = "%lo(";
+ 	}
+ 
+       if (TARGET_XGOT)
+ 	{
+ 	  /* The HIGH and LO_SUM are matched by special .md patterns.  */
+ 	  mips_split_p[SYMBOL_GOT_DISP] = true;
+ 
+ 	  mips_split_p[SYMBOL_GOTOFF_DISP] = true;
+ 	  mips_hi_relocs[SYMBOL_GOTOFF_DISP] = "%got_hi(";
+ 	  mips_lo_relocs[SYMBOL_GOTOFF_DISP] = "%got_lo(";
+ 
+ 	  mips_split_p[SYMBOL_GOTOFF_CALL] = true;
+ 	  mips_hi_relocs[SYMBOL_GOTOFF_CALL] = "%call_hi(";
+ 	  mips_lo_relocs[SYMBOL_GOTOFF_CALL] = "%call_lo(";
+ 	}
+       else
+ 	{
+ 	  if (TARGET_NEWABI)
+ 	    mips_lo_relocs[SYMBOL_GOTOFF_DISP] = "%got_disp(";
+ 	  else
+ 	    mips_lo_relocs[SYMBOL_GOTOFF_DISP] = "%got(";
+ 	  mips_lo_relocs[SYMBOL_GOTOFF_CALL] = "%call16(";
+ 	}
+     }
+ 
+   if (TARGET_NEWABI)
+     {
+       mips_split_p[SYMBOL_GOTOFF_LOADGP] = true;
+       mips_hi_relocs[SYMBOL_GOTOFF_LOADGP] = "%hi(%neg(%gp_rel(";
+       mips_lo_relocs[SYMBOL_GOTOFF_LOADGP] = "%lo(%neg(%gp_rel(";
+     }
+ 
+   /* Thread-local relocation operators.  */
+   mips_lo_relocs[SYMBOL_TLSGD] = "%tlsgd(";
+   mips_lo_relocs[SYMBOL_TLSLDM] = "%tlsldm(";
+   mips_split_p[SYMBOL_DTPREL] = 1;
+   mips_hi_relocs[SYMBOL_DTPREL] = "%dtprel_hi(";
+   mips_lo_relocs[SYMBOL_DTPREL] = "%dtprel_lo(";
+   mips_lo_relocs[SYMBOL_GOTTPREL] = "%gottprel(";
+   mips_split_p[SYMBOL_TPREL] = 1;
+   mips_hi_relocs[SYMBOL_TPREL] = "%tprel_hi(";
+   mips_lo_relocs[SYMBOL_TPREL] = "%tprel_lo(";
+ 
+   mips_lo_relocs[SYMBOL_HALF] = "%half(";
+ }
+ 
+ static GTY(()) int was_mips16_p = -1;
+ 
+ /* Set up the target-dependent global state so that it matches the
+    current function's ISA mode.  */
+ static void
+ mips_set_mips16_mode (int mips16_p)
+ {
+   if (mips16_p == was_mips16_p)
+     return;
+ 
+   /* Restore base settings of various flags.  */
+   target_flags = mips_base_target_flags;
+   align_loops = mips_base_align_loops;
+   align_jumps = mips_base_align_jumps;
+   align_functions = mips_base_align_functions;
+   flag_schedule_insns = mips_base_schedule_insns;
+   flag_reorder_blocks_and_partition = mips_base_reorder_blocks_and_partition;
+   flag_delayed_branch = mips_flag_delayed_branch;
+   
+   if (mips16_p) 
+     {
+       /* Select mips16 instruction set.  */
+       target_flags |= MASK_MIPS16;
+ 
+       /* Don't run the scheduler before reload, since it tends to
+          increase register pressure.  */
+       flag_schedule_insns = 0;
+ 
+       /* Don't do hot/cold partitioning.  The constant layout code expects
+        the whole function to be in a single section.  */
+       flag_reorder_blocks_and_partition = 0;
+ 
+       /* Silently disable -mexplicit-relocs since it doesn't apply
+ 	 to mips16 code.  Even so, it would overly pedantic to warn
+ 	 about "-mips16 -mexplicit-relocs", especially given that
+ 	 we use a %gprel() operator.  */
+       target_flags &= ~MASK_EXPLICIT_RELOCS;
+ 
+       /* Silently disable DSP extensions.  We already complained in
+ 	 override_options if both -mdsp and -mips16 were specified on the
+ 	 command line, so here we're assuming local mips16 function
+ 	 attribute takes precedence over command-line -mdsp.  */
+       target_flags &= ~MASK_DSP;
+       target_flags &= ~MASK_DSPR2;
+ 
+       /* Force no alignment of mips16 code.  */
+       /* XXX would 32-bit alignment be an acceptable compromise?  */
+       align_loops = align_jumps = align_functions = 1;
+ 
+       /* We don't have a thread pointer access instruction on MIPS16, or
+ 	 appropriate TLS relocations.  */
+       targetm.have_tls = false;
+     }
+   else 
+     {
+       /* Reset to select base non-mips16 ISA.  */
+       target_flags &= ~MASK_MIPS16;
+ 
+       /* When using explicit relocs, we call dbr_schedule from within
+ 	 mips_reorg.  */
+       if (TARGET_EXPLICIT_RELOCS)
+ 	flag_delayed_branch = 0;
+ 
+       /* TLS is only supported for per-file nomips16 processing, not
+ 	 per-function, so we can correctly generate the initialization
+ 	 code for TLS variables. */
+       targetm.have_tls = TARGET_HAVE_TLS && !mips_base_mips16;
+     }
+ 
+   /* (Re)initialise mips target internals for new ISA.  */
+   mips_init_split_addresses ();
+   mips_init_relocs ();
+ 
+   if (was_mips16_p >= 0)
+     /* Reinitialize target-dependent state.  */
+     target_reinit ();
+ 
+   was_mips16_p = TARGET_MIPS16;
+ }
+ 
+ /* Implement TARGET_SET_CURRENT_FUNCTION.  Decide whether the current 
+    function should use the MIPS16 ISA and switch modes accordingly.  */
+ 
+ static void
+ mips_set_current_function (tree fndecl)
+ {
+   int mips16p;
+   if (fndecl)
+     mips16p = SYMBOL_REF_MIPS16_FUNC_P (XEXP (DECL_RTL (fndecl), 0));
+   else
+     mips16p = mips_base_mips16 != 0;
+   mips_set_mips16_mode (mips16p);
+ }
+ 
  /* Implement TARGET_HANDLE_OPTION.  */
  
  static bool
*************** override_options (void)
*** 5258,5263 ****
--- 5534,5547 ----
        target_flags &= ~MASK_ABICALLS;
      }
  
+   /* MIPS16 cannot generate PIC yet.  */
+   if (TARGET_MIPS16 && (flag_pic || TARGET_ABICALLS))
+     {
+       sorry ("MIPS16 PIC");
+       target_flags &= ~MASK_ABICALLS;
+       flag_pic = flag_pie = flag_shlib = 0;
+     }
+ 
    if (TARGET_ABICALLS)
      {
        /* We need to set flag_pic for executables as well as DSOs
*************** override_options (void)
*** 5275,5296 ****
    if (TARGET_VXWORKS_RTP && mips_section_threshold > 0)
      warning (0, "-G and -mrtp are incompatible");
  
-   /* mips_split_addresses is a half-way house between explicit
-      relocations and the traditional assembler macros.  It can
-      split absolute 32-bit symbolic constants into a high/lo_sum
-      pair but uses macros for other sorts of access.
- 
-      Like explicit relocation support for REL targets, it relies
-      on GNU extensions in the assembler and the linker.
- 
-      Although this code should work for -O0, it has traditionally
-      been treated as an optimization.  */
-   if (!TARGET_MIPS16 && TARGET_SPLIT_ADDRESSES
-       && optimize && !flag_pic
-       && !ABI_HAS_64BIT_SYMBOLS)
-     mips_split_addresses = 1;
-   else
-     mips_split_addresses = 0;
  
    /* -mvr4130-align is a "speed over size" optimization: it usually produces
       faster code, but at the expense of more nops.  Enable it at -O3 and
--- 5559,5564 ----
*************** override_options (void)
*** 5487,5493 ****
    gpr_mode = TARGET_64BIT ? DImode : SImode;
  
    /* Provide default values for align_* for 64-bit targets.  */
!   if (TARGET_64BIT && !TARGET_MIPS16)
      {
        if (align_loops == 0)
  	align_loops = 8;
--- 5755,5761 ----
    gpr_mode = TARGET_64BIT ? DImode : SImode;
  
    /* Provide default values for align_* for 64-bit targets.  */
!   if (TARGET_64BIT)
      {
        if (align_loops == 0)
  	align_loops = 8;
*************** override_options (void)
*** 5500,5610 ****
    /* Function to allocate machine-dependent function status.  */
    init_machine_status = &mips_init_machine_status;
  
-   if (ABI_HAS_64BIT_SYMBOLS)
-     {
-       if (TARGET_EXPLICIT_RELOCS)
- 	{
- 	  mips_split_p[SYMBOL_64_HIGH] = true;
- 	  mips_hi_relocs[SYMBOL_64_HIGH] = "%highest(";
- 	  mips_lo_relocs[SYMBOL_64_HIGH] = "%higher(";
- 
- 	  mips_split_p[SYMBOL_64_MID] = true;
- 	  mips_hi_relocs[SYMBOL_64_MID] = "%higher(";
- 	  mips_lo_relocs[SYMBOL_64_MID] = "%hi(";
- 
- 	  mips_split_p[SYMBOL_64_LOW] = true;
- 	  mips_hi_relocs[SYMBOL_64_LOW] = "%hi(";
- 	  mips_lo_relocs[SYMBOL_64_LOW] = "%lo(";
- 
- 	  mips_split_p[SYMBOL_ABSOLUTE] = true;
- 	  mips_lo_relocs[SYMBOL_ABSOLUTE] = "%lo(";
- 	}
-     }
-   else
-     {
-       if (TARGET_EXPLICIT_RELOCS || mips_split_addresses || TARGET_MIPS16)
- 	{
- 	  mips_split_p[SYMBOL_ABSOLUTE] = true;
- 	  mips_hi_relocs[SYMBOL_ABSOLUTE] = "%hi(";
- 	  mips_lo_relocs[SYMBOL_ABSOLUTE] = "%lo(";
- 
- 	  mips_lo_relocs[SYMBOL_32_HIGH] = "%hi(";
- 	}
-     }
- 
-   if (TARGET_MIPS16)
-     {
-       /* The high part is provided by a pseudo copy of $gp.  */
-       mips_split_p[SYMBOL_GP_RELATIVE] = true;
-       mips_lo_relocs[SYMBOL_GP_RELATIVE] = "%gprel(";
-     }
- 
-   if (TARGET_EXPLICIT_RELOCS)
-     {
-       /* Small data constants are kept whole until after reload,
- 	 then lowered by mips_rewrite_small_data.  */
-       mips_lo_relocs[SYMBOL_GP_RELATIVE] = "%gp_rel(";
- 
-       mips_split_p[SYMBOL_GOT_PAGE_OFST] = true;
-       if (TARGET_NEWABI)
- 	{
- 	  mips_lo_relocs[SYMBOL_GOTOFF_PAGE] = "%got_page(";
- 	  mips_lo_relocs[SYMBOL_GOT_PAGE_OFST] = "%got_ofst(";
- 	}
-       else
- 	{
- 	  mips_lo_relocs[SYMBOL_GOTOFF_PAGE] = "%got(";
- 	  mips_lo_relocs[SYMBOL_GOT_PAGE_OFST] = "%lo(";
- 	}
- 
-       if (TARGET_XGOT)
- 	{
- 	  /* The HIGH and LO_SUM are matched by special .md patterns.  */
- 	  mips_split_p[SYMBOL_GOT_DISP] = true;
- 
- 	  mips_split_p[SYMBOL_GOTOFF_DISP] = true;
- 	  mips_hi_relocs[SYMBOL_GOTOFF_DISP] = "%got_hi(";
- 	  mips_lo_relocs[SYMBOL_GOTOFF_DISP] = "%got_lo(";
- 
- 	  mips_split_p[SYMBOL_GOTOFF_CALL] = true;
- 	  mips_hi_relocs[SYMBOL_GOTOFF_CALL] = "%call_hi(";
- 	  mips_lo_relocs[SYMBOL_GOTOFF_CALL] = "%call_lo(";
- 	}
-       else
- 	{
- 	  if (TARGET_NEWABI)
- 	    mips_lo_relocs[SYMBOL_GOTOFF_DISP] = "%got_disp(";
- 	  else
- 	    mips_lo_relocs[SYMBOL_GOTOFF_DISP] = "%got(";
- 	  mips_lo_relocs[SYMBOL_GOTOFF_CALL] = "%call16(";
- 	}
-     }
- 
-   if (TARGET_NEWABI)
-     {
-       mips_split_p[SYMBOL_GOTOFF_LOADGP] = true;
-       mips_hi_relocs[SYMBOL_GOTOFF_LOADGP] = "%hi(%neg(%gp_rel(";
-       mips_lo_relocs[SYMBOL_GOTOFF_LOADGP] = "%lo(%neg(%gp_rel(";
-     }
- 
-   /* Thread-local relocation operators.  */
-   mips_lo_relocs[SYMBOL_TLSGD] = "%tlsgd(";
-   mips_lo_relocs[SYMBOL_TLSLDM] = "%tlsldm(";
-   mips_split_p[SYMBOL_DTPREL] = 1;
-   mips_hi_relocs[SYMBOL_DTPREL] = "%dtprel_hi(";
-   mips_lo_relocs[SYMBOL_DTPREL] = "%dtprel_lo(";
-   mips_lo_relocs[SYMBOL_GOTTPREL] = "%gottprel(";
-   mips_split_p[SYMBOL_TPREL] = 1;
-   mips_hi_relocs[SYMBOL_TPREL] = "%tprel_hi(";
-   mips_lo_relocs[SYMBOL_TPREL] = "%tprel_lo(";
- 
-   mips_lo_relocs[SYMBOL_HALF] = "%half(";
- 
-   /* We don't have a thread pointer access instruction on MIPS16, or
-      appropriate TLS relocations.  */
-   if (TARGET_MIPS16)
-     targetm.have_tls = false;
- 
    /* Default to working around R4000 errata only if the processor
       was selected explicitly.  */
    if ((target_flags_explicit & MASK_FIX_R4000) == 0
--- 5768,5773 ----
*************** override_options (void)
*** 5616,5621 ****
--- 5779,5795 ----
    if ((target_flags_explicit & MASK_FIX_R4400) == 0
        && mips_matching_cpu_name_p (mips_arch_info->name, "r4400"))
      target_flags |= MASK_FIX_R4400;
+ 
+   /* Save base state of options, so that we can later select mips16 or
+      nomips16 mode, as appropriate.  See mips_set_current_function.  */
+   mips_base_mips16 = target_flags & MASK_MIPS16;
+   mips_base_target_flags = target_flags;
+   mips_base_schedule_insns = flag_schedule_insns;
+   mips_base_reorder_blocks_and_partition = flag_reorder_blocks_and_partition;
+   mips_base_align_loops = align_loops;
+   mips_base_align_jumps = align_jumps;
+   mips_base_align_functions = align_functions;
+   mips_flag_delayed_branch = flag_delayed_branch;
  }
  
  /* Swap the register information for registers I and I + 1, which
*************** mips_file_start (void)
*** 6371,6379 ****
    if (TARGET_ABICALLS)
      fprintf (asm_out_file, "\t.abicalls\n");
  
-   if (TARGET_MIPS16)
-     fprintf (asm_out_file, "\t.set\tmips16\n");
- 
    if (flag_verbose_asm)
      fprintf (asm_out_file, "\n%s -G value = %d, Arch = %s, ISA = %d\n",
  	     ASM_COMMENT_START,
--- 6545,6550 ----
*************** mips_output_function_prologue (FILE *fil
*** 7228,7233 ****
--- 7399,7410 ----
        && current_function_args_info.fp_code != 0)
      build_mips16_function_stub (file);
  
+   /* Select the mips16 mode for this function.  */
+   if (TARGET_MIPS16)
+     fprintf (file, "\t.set\tmips16\n");
+   else 
+     fprintf (file, "\t.set\tnomips16\n");
+ 
    if (!FUNCTION_NAME_ALREADY_DECLARED)
      {
        /* Get the function name the same way that toplev.c does before calling
*************** mips_output_mi_thunk (FILE *file, tree t
*** 8184,8190 ****
  	TARGET_CALL_SAVED_GP ? 15 : GLOBAL_POINTER_REGNUM;
  
        SET_REGNO (pic_offset_table_rtx, cfun->machine->global_pointer);
- 
      }
  
    /* Set up the global pointer for n32 or n64 abicalls.  If
--- 8361,8366 ----
*************** mips_output_mi_thunk (FILE *file, tree t
*** 8235,8241 ****
    /* Jump to the target function.  Use a sibcall if direct jumps are
       allowed, otherwise load the address into a register first.  */
    fnaddr = XEXP (DECL_RTL (function), 0);
!   if (TARGET_MIPS16 || TARGET_USE_GOT || SYMBOL_REF_LONG_CALL_P (fnaddr))
      {
        /* This is messy.  gas treats "la $25,foo" as part of a call
  	 sequence and may allow a global "foo" to be lazily bound.
--- 8411,8418 ----
    /* Jump to the target function.  Use a sibcall if direct jumps are
       allowed, otherwise load the address into a register first.  */
    fnaddr = XEXP (DECL_RTL (function), 0);
!   if (TARGET_MIPS16 || TARGET_USE_GOT || SYMBOL_REF_LONG_CALL_P (fnaddr)
!       || SYMBOL_REF_MIPS16_FUNC_P (fnaddr))
      {
        /* This is messy.  gas treats "la $25,foo" as part of a call
  	 sequence and may allow a global "foo" to be lazily bound.
*************** build_mips16_function_stub (FILE *file)
*** 9071,9078 ****
        fputs ("\n", file);
      }
  
-   fprintf (file, "\t.set\tmips16\n");
- 
    switch_to_section (function_section (current_function_decl));
  }
  
--- 9248,9253 ----
*************** build_mips16_call_stub (rtx retval, rtx 
*** 9404,9411 ****
  	  fputs ("\n", asm_out_file);
  	}
  
-       fprintf (asm_out_file, "\t.set\tmips16\n");
- 
        /* Record this stub.  */
        l = (struct mips16_stub *) xmalloc (sizeof *l);
        l->name = xstrdup (fnname);
--- 9579,9584 ----
*************** mips_expand_builtin (tree exp, rtx targe
*** 11646,11651 ****
--- 11819,11831 ----
    fndecl = TREE_OPERAND (CALL_EXPR_FN (exp), 0);
    fcode = DECL_FUNCTION_CODE (fndecl);
  
+   if (TARGET_MIPS16)
+     {
+       error ("built-in function `%s' not supported for MIPS16",
+ 	     IDENTIFIER_POINTER (DECL_NAME (fndecl)));
+       return const0_rtx;
+     }
+ 
    bdesc = NULL;
    for (m = bdesc_arrays; m < &bdesc_arrays[ARRAY_SIZE (bdesc_arrays)]; m++)
      {
*************** mips_expand_builtin_bposge (enum mips_bu
*** 12152,12157 ****
--- 12332,12379 ----
  				       const1_rtx, const0_rtx);
  }
  \f
+ /* Return true if we should force MIPS16 mode for the function named by
+    the SYMBOL_REF SYMBOL, which belongs to DECL and has type TYPE.
+    FIRST is true if this is the first time handling this decl.  */
+ static int
+ mips_use_mips16_mode_p (rtx symbol, tree decl, int first, tree type)
+ {
+   int is_mips16 = mips_base_mips16;
+   tree parent;
+ 
+   if (mips_mips16_type_p (type))
+     is_mips16 = 1;
+   else if (mips_nomips16_type_p (type))
+     is_mips16 = 0;
+   else if (parent = decl_function_context (decl))
+     /* Nested function should inherit MIPS16 setting from its parent.  */
+     is_mips16 = SYMBOL_REF_MIPS16_FUNC_P (XEXP (DECL_RTL (parent), 0));
+   else if (TARGET_FLIP_MIPS16
+ 	   && !DECL_BUILT_IN (decl)
+ 	   && !DECL_ARTIFICIAL (decl))
+     {
+       if (first)
+ 	{
+ 	  /* Debug: flip MIPS16 on each function. */
+ 	  mips16_flipper = !mips16_flipper;
+ 	  if (mips16_flipper)
+ 	    is_mips16 = !is_mips16;
+ 	}
+       else
+ 	/* Don't flip after first again. */
+ 	is_mips16 = SYMBOL_REF_MIPS16_FUNC_P (symbol);
+     }
+ 
+   /* If there was an explicit -mno-mips16, then prevent MIPS16
+      selection, even when an explicit attribute is given.  */
+   if ((target_flags_explicit & MASK_MIPS16)
+       && mips_base_mips16 == 0)
+     is_mips16 = 0;
+ 
+   return is_mips16;
+ }
+ 
+ 
  /* Set SYMBOL_REF_FLAGS for the SYMBOL_REF inside RTL, which belongs to DECL.
     FIRST is true if this is the first time handling this decl.  */
  
*************** mips_encode_section_info (tree decl, rtx
*** 12163,12172 ****
    if (TREE_CODE (decl) == FUNCTION_DECL)
      {
        rtx symbol = XEXP (rtl, 0);
  
!       if ((TARGET_LONG_CALLS && !mips_near_type_p (TREE_TYPE (decl)))
! 	  || mips_far_type_p (TREE_TYPE (decl)))
  	SYMBOL_REF_FLAGS (symbol) |= SYMBOL_FLAG_LONG_CALL;
      }
  }
  
--- 12385,12403 ----
    if (TREE_CODE (decl) == FUNCTION_DECL)
      {
        rtx symbol = XEXP (rtl, 0);
+       tree type = TREE_TYPE (decl);
  
!       if ((TARGET_LONG_CALLS && !mips_near_type_p (type))
! 	  || mips_far_type_p (type))
  	SYMBOL_REF_FLAGS (symbol) |= SYMBOL_FLAG_LONG_CALL;
+ 
+       if (mips_use_mips16_mode_p (symbol, decl, first, type))
+ 	{
+ 	  if (flag_pic || TARGET_ABICALLS)
+ 	    sorry ("MIPS16 PIC");
+ 	  else
+ 	    SYMBOL_REF_FLAGS (symbol) |= SYMBOL_FLAG_MIPS16_FUNC;
+ 	}
      }
  }
  
Index: gcc/doc/extend.texi
===================================================================
*** gcc/doc/extend.texi	(revision 127832)
--- gcc/doc/extend.texi	(working copy)
*************** long as the old pointer is never referre
*** 2191,2196 ****
--- 2191,2214 ----
  to the new pointer) after the function returns a non-@code{NULL}
  value.
  
+ @item mips16/nomips16
+ @cindex @code{mips16} attribute
+ @cindex @code{nomips16} attribute
+ 
+ On MIPS targets, you can use the @code{mips16} and @code{nomips16}
+ function attributes to locally select or turn off MIPS16 code generation.
+ A function with the @code{mips16} attribute is emitted as MIPS16 code
+ unless an explicit @option{-mno-mips16} option was specified on the command
+ line.  MIPS16 code generation is disabled for functions with the
+ @code{nomips16} attribute, even if @option{-mips16} was specified on the
+ command line.  @xref{MIPS Options}, for more details about these options.
+ 
+ When compiling files containing mixed MIPS16 and non-MIPS16 code, the
+ preprocessor symbol @code{__mips16} reflects the setting on the command line,
+ not that within individual functions.  Mixed MIPS16 and non-MIPS16 code
+ may interact badly with some GCC extensions such as @code{__builtin_apply}
+ (@pxref{Constructing Calls}).
+ 
  @item model (@var{model-name})
  @cindex function addressability on the M32R/D
  @cindex variable addressability on the IA-64
Index: gcc/doc/invoke.texi
===================================================================
*** gcc/doc/invoke.texi	(revision 127832)
--- gcc/doc/invoke.texi	(working copy)
*************** Objective-C and Objective-C++ Dialects}.
*** 620,626 ****
  @emph{MIPS Options}
  @gccoptlist{-EL  -EB  -march=@var{arch}  -mtune=@var{arch} @gol
  -mips1  -mips2  -mips3  -mips4  -mips32  -mips32r2  -mips64 @gol
! -mips16  -mno-mips16  -mabi=@var{abi}  -mabicalls  -mno-abicalls @gol
  -mshared  -mno-shared  -mxgot  -mno-xgot  -mgp32  -mgp64 @gol
  -mfp32  -mfp64  -mhard-float  -msoft-float @gol
  -msingle-float  -mdouble-float  -mdsp  -mno-dsp  -mdspr2  -mno-dspr2 @gol
--- 620,627 ----
  @emph{MIPS Options}
  @gccoptlist{-EL  -EB  -march=@var{arch}  -mtune=@var{arch} @gol
  -mips1  -mips2  -mips3  -mips4  -mips32  -mips32r2  -mips64 @gol
! -mips16  -mno-mips16  -mflip-mips16 @gol
! -mabi=@var{abi}  -mabicalls  -mno-abicalls @gol
  -mshared  -mno-shared  -mxgot  -mno-xgot  -mgp32  -mgp64 @gol
  -mfp32  -mfp64  -mhard-float  -msoft-float @gol
  -msingle-float  -mdouble-float  -mdsp  -mno-dsp  -mdspr2  -mno-dspr2 @gol
*************** Equivalent to @samp{-march=mips64}.
*** 11568,11573 ****
--- 11569,11584 ----
  Generate (do not generate) MIPS16 code.  If GCC is targetting a
  MIPS32 or MIPS64 architecture, it will make use of the MIPS16e ASE@.
  
+ MIPS16 code generation can also be controlled on a per-function basis
+ by means of @code{mips16} and @code{nomips16} attributes.  
+ @xref{Function Attributes}, for more information.
+ 
+ @item -mflip-mips16
+ @opindex mflip-mips16
+ Generate MIPS16 code on alternating functions.  This option is provided
+ for regression testing of mixed MIPS16/non-MIPS16 code generation, and is
+ not intended for ordinary use in compiling user code.
+ 
  @item -mabi=32
  @itemx -mabi=o64
  @itemx -mabi=n32
Index: gcc/testsuite/gcc.target/mips/mips16-attributes.c
===================================================================
*** gcc/testsuite/gcc.target/mips/mips16-attributes.c	(revision 0)
--- gcc/testsuite/gcc.target/mips/mips16-attributes.c	(revision 0)
***************
*** 0 ****
--- 1,82 ----
+ /* Verify that mips16 and nomips16 attributes work, checking all combinations
+    of calling a nomips16/mips16/default function from a nomips16/mips16/default
+    function.  */
+ /* { dg-do run } */
+ 
+ #include <stdlib.h>
+ 
+ #define ATTR1 __attribute__ ((nomips16))
+ #define ATTR2 __attribute__ ((mips16))
+ #define ATTR3
+ 
+ double ATTR1
+ f1 (int i, float f, double d)
+ {
+   return i + f + d;
+ }
+ 
+ double ATTR2
+ f2 (int i, float f, double d)
+ {
+   return i + f + d;
+ }
+ 
+ double ATTR3
+ f3 (int i, float f, double d)
+ {
+   return i + f + d;
+ }
+ 
+ void ATTR1
+ g1 (int i, float f, double d)
+ {
+   double r = i + f + d;
+ 
+   if (f1 (i, f, d) != r)
+     abort ();
+   if (f2 (i+1, f+1, d+1) != r + 3)
+     abort ();
+   if (f3 (i+2, f+2, d+2) != r + 6)
+     abort ();
+ }
+ 
+ void ATTR2
+ g2 (int i, float f, double d)
+ {
+   double r = i + f + d;
+ 
+   if (f1 (i, f, d) != r)
+     abort ();
+   if (f2 (i+1, f+1, d+1) != r + 3)
+     abort ();
+   if (f3 (i+2, f+2, d+2) != r + 6)
+     abort ();
+ }
+ 
+ void ATTR3
+ g3 (int i, float f, double d)
+ {
+   double r = i + f + d;
+ 
+   if (f1 (i, f, d) != r)
+     abort ();
+   if (f2 (i+1, f+1, d+1) != r + 3)
+     abort ();
+   if (f3 (i+2, f+2, d+2) != r + 6)
+     abort ();
+ }
+ 
+ int ATTR3
+ main (void)
+ {
+   int i = 1;
+   float f = -2.0;
+   double d = 3.0;
+ 
+   g1 (i, f, d);
+   g2 (i, f, d);
+   g3 (i, f, d);
+ 
+   exit (0);
+ }
+ 
Index: gcc/testsuite/gcc.c-torture/compile/mipscop-1.c
===================================================================
*** gcc/testsuite/gcc.c-torture/compile/mipscop-1.c	(revision 127832)
--- gcc/testsuite/gcc.c-torture/compile/mipscop-1.c	(working copy)
***************
*** 1,9 ****
  /* { dg-do compile { target mips*-*-* } } */
  
- #ifndef __mips16
  register unsigned int cp0count asm ("$c0r1");
  
! int
  main (int argc, char *argv[])
  {
    unsigned int d;
--- 1,8 ----
  /* { dg-do compile { target mips*-*-* } } */
  
  register unsigned int cp0count asm ("$c0r1");
  
! int __attribute__ ((nomips16))
  main (int argc, char *argv[])
  {
    unsigned int d;
*************** main (int argc, char *argv[])
*** 11,14 ****
    d = cp0count + 3;
    printf ("%d\n", d);
  }
- #endif
--- 10,12 ----
Index: gcc/testsuite/gcc.c-torture/compile/mipscop-2.c
===================================================================
*** gcc/testsuite/gcc.c-torture/compile/mipscop-2.c	(revision 127832)
--- gcc/testsuite/gcc.c-torture/compile/mipscop-2.c	(working copy)
***************
*** 1,11 ****
  /* { dg-do compile { target mips*-*-* } } */
  
- #ifndef __mips16
  register unsigned int c3r1 asm ("$c3r1");
  
  extern unsigned int b, c;
  
! void
  foo ()
  {
    unsigned int a, d;
--- 1,10 ----
  /* { dg-do compile { target mips*-*-* } } */
  
  register unsigned int c3r1 asm ("$c3r1");
  
  extern unsigned int b, c;
  
! void __attribute__ ((nomips16))
  foo ()
  {
    unsigned int a, d;
*************** foo ()
*** 17,20 ****
    d = c3r1;
    printf ("%d\n", d);
  }
- #endif
--- 16,18 ----
Index: gcc/testsuite/gcc.c-torture/compile/mipscop-3.c
===================================================================
*** gcc/testsuite/gcc.c-torture/compile/mipscop-3.c	(revision 127832)
--- gcc/testsuite/gcc.c-torture/compile/mipscop-3.c	(working copy)
***************
*** 1,11 ****
  /* { dg-do compile { target mips*-*-* } } */
  
- #ifndef __mips16
  register unsigned int c3r1 asm ("$c3r1"), c3r2 asm ("$c3r2");
  
  extern unsigned int b, c;
  
! void
  foo ()
  {
    unsigned int a, d;
--- 1,10 ----
  /* { dg-do compile { target mips*-*-* } } */
  
  register unsigned int c3r1 asm ("$c3r1"), c3r2 asm ("$c3r2");
  
  extern unsigned int b, c;
  
! void __attribute__ ((nomips16))
  foo ()
  {
    unsigned int a, d;
*************** foo ()
*** 17,20 ****
    d = c3r1;
    printf ("%d\n", d);
  }
- #endif
--- 16,18 ----
Index: gcc/testsuite/gcc.c-torture/compile/mipscop-4.c
===================================================================
*** gcc/testsuite/gcc.c-torture/compile/mipscop-4.c	(revision 127832)
--- gcc/testsuite/gcc.c-torture/compile/mipscop-4.c	(working copy)
***************
*** 1,11 ****
  /* { dg-do compile { target mips*-*-* } } */
  
- #ifndef __mips16
  register unsigned long c3r1 asm ("$c3r1"), c3r2 asm ("$c3r2");
  
  extern unsigned long b, c;
  
! void
  foo ()
  {
    unsigned long a, d;
--- 1,10 ----
  /* { dg-do compile { target mips*-*-* } } */
  
  register unsigned long c3r1 asm ("$c3r1"), c3r2 asm ("$c3r2");
  
  extern unsigned long b, c;
  
! void __attribute__ ((nomips16))
  foo ()
  {
    unsigned long a, d;
*************** foo ()
*** 17,20 ****
    d = c3r1;
    printf ("%d\n", d);
  }
! #endif
--- 16,19 ----
    d = c3r1;
    printf ("%d\n", d);
  }
! 
Index: gcc/testsuite/gcc.dg/torture/mips-hilo-1.c
===================================================================
*** gcc/testsuite/gcc.dg/torture/mips-hilo-1.c	(revision 127832)
--- gcc/testsuite/gcc.dg/torture/mips-hilo-1.c	(working copy)
***************
*** 6,15 ****
  extern void abort (void);
  extern void exit (int);
  
- #if !defined(__mips16)
- 
  #define DECLARE(TYPE)							\
!   TYPE __attribute__ ((noinline))					\
    f1##TYPE (TYPE x1, TYPE x2, TYPE x3)					\
    {									\
      TYPE t1, t2;							\
--- 6,13 ----
  extern void abort (void);
  extern void exit (int);
  
  #define DECLARE(TYPE)							\
!   TYPE __attribute__ ((noinline)) __attribute__ ((nomips16))		\
    f1##TYPE (TYPE x1, TYPE x2, TYPE x3)					\
    {									\
      TYPE t1, t2;							\
*************** extern void exit (int);
*** 19,25 ****
      return t1 + t2;							\
    }									\
  									\
!   TYPE __attribute__ ((noinline))					\
    f2##TYPE (TYPE x1, TYPE x2, TYPE x3)					\
    {									\
      TYPE t1, t2;							\
--- 17,23 ----
      return t1 + t2;							\
    }									\
  									\
!   TYPE __attribute__ ((noinline)) __attribute__ ((nomips16))		\
    f2##TYPE (TYPE x1, TYPE x2, TYPE x3)					\
    {									\
      TYPE t1, t2;							\
*************** main ()
*** 73,78 ****
  #endif
    exit (0);
  }
- #else
- int main () { exit (0); }
- #endif
--- 71,73 ----
Index: gcc/testsuite/gcc.dg/torture/mips-hilo-2.c
===================================================================
*** gcc/testsuite/gcc.dg/torture/mips-hilo-2.c	(revision 127832)
--- gcc/testsuite/gcc.dg/torture/mips-hilo-2.c	(working copy)
***************
*** 5,14 ****
  extern void abort (void);
  extern void exit (int);
  
- #if !defined(__mips16)
  unsigned int g;
  
! unsigned long long f (unsigned int x)
  {
    union { unsigned long long ll; unsigned int parts[2]; } u;
  
--- 5,13 ----
  extern void abort (void);
  extern void exit (int);
  
  unsigned int g;
  
! unsigned __attribute__ ((nomips16)) long long f (unsigned int x)
  {
    union { unsigned long long ll; unsigned int parts[2]; } u;
  
*************** unsigned long long f (unsigned int x)
*** 17,23 ****
    return u.ll;
  }
  
! int main ()
  {
    union { unsigned long long ll; unsigned int parts[2]; } u;
  
--- 16,22 ----
    return u.ll;
  }
  
! int __attribute__ ((nomips16)) main ()
  {
    union { unsigned long long ll; unsigned int parts[2]; } u;
  
*************** int main ()
*** 26,31 ****
      abort ();
    exit (0);
  }
- #else
- int main () { exit (0); }
- #endif
--- 25,27 ----
Index: gcc/testsuite/gcc.dg/torture/pr19683-1.c
===================================================================
*** gcc/testsuite/gcc.dg/torture/pr19683-1.c	(revision 127832)
--- gcc/testsuite/gcc.dg/torture/pr19683-1.c	(working copy)
***************
*** 6,12 ****
  extern void abort (void);
  extern void exit (int);
  
- #ifndef __mips16
  #define REPEAT10(X, Y)					\
    X(Y##0); X(Y##1); X(Y##2); X(Y##3); X(Y##4);		\
    X(Y##5); X(Y##6); X(Y##7); X(Y##8); X(Y##9)
--- 6,11 ----
*************** extern void exit (int);
*** 17,23 ****
  
  union u { unsigned long long ll; unsigned int i[2]; };
  
! unsigned int
  foo (volatile unsigned int *ptr)
  {
    union u u;
--- 16,22 ----
  
  union u { unsigned long long ll; unsigned int i[2]; };
  
! unsigned int __attribute__ ((nomips16))
  foo (volatile unsigned int *ptr)
  {
    union u u;
*************** foo (volatile unsigned int *ptr)
*** 30,36 ****
    return result;
  }
  
! int
  main (void)
  {
    unsigned int array[] = { 1000 * 1000 * 1000 };
--- 29,35 ----
    return result;
  }
  
! int __attribute__ ((nomips16))
  main (void)
  {
    unsigned int array[] = { 1000 * 1000 * 1000 };
*************** main (void)
*** 41,50 ****
      abort ();
    exit (0);
  }
- #else
- int
- main (void)
- {
-   exit (0);
- }
- #endif
--- 40,42 ----
Index: gcc/testsuite/gcc.target/mips/madd-3.c
===================================================================
*** gcc/testsuite/gcc.target/mips/madd-3.c	(revision 127832)
--- gcc/testsuite/gcc.target/mips/madd-3.c	(working copy)
***************
*** 2,20 ****
  /* { dg-mips-options "-O2 -mips32 -mgp32" } */
  /* { dg-final { scan-assembler-times "\tmadd\t" 3 } } */
  
! long long
  f1 (int x, int y, long long z)
  {
    return (long long) x * y + z;
  }
  
! long long
  f2 (int x, int y, long long z)
  {
    return z + (long long) y * x;
  }
  
! long long
  f3 (int x, int y, long long z)
  {
    long long t = (long long) x * y;
--- 2,20 ----
  /* { dg-mips-options "-O2 -mips32 -mgp32" } */
  /* { dg-final { scan-assembler-times "\tmadd\t" 3 } } */
  
! long long __attribute__ ((nomips16))
  f1 (int x, int y, long long z)
  {
    return (long long) x * y + z;
  }
  
! long long __attribute__ ((nomips16))
  f2 (int x, int y, long long z)
  {
    return z + (long long) y * x;
  }
  
! long long __attribute__ ((nomips16))
  f3 (int x, int y, long long z)
  {
    long long t = (long long) x * y;
Index: gcc/testsuite/gcc.target/mips/maddu-3.c
===================================================================
*** gcc/testsuite/gcc.target/mips/maddu-3.c	(revision 127832)
--- gcc/testsuite/gcc.target/mips/maddu-3.c	(working copy)
***************
*** 5,23 ****
  typedef unsigned int ui;
  typedef unsigned long long ull;
  
! ull
  f1 (ui x, ui y, ull z)
  {
    return (ull) x * y + z;
  }
  
! ull
  f2 (ui x, ui y, ull z)
  {
    return z + (ull) y * x;
  }
  
! ull
  f3 (ui x, ui y, ull z)
  {
    ull t = (ull) x * y;
--- 5,23 ----
  typedef unsigned int ui;
  typedef unsigned long long ull;
  
! ull __attribute__ ((nomips16))
  f1 (ui x, ui y, ull z)
  {
    return (ull) x * y + z;
  }
  
! ull __attribute__ ((nomips16))
  f2 (ui x, ui y, ull z)
  {
    return z + (ull) y * x;
  }
  
! ull __attribute__ ((nomips16))
  f3 (ui x, ui y, ull z)
  {
    ull t = (ull) x * y;
Index: gcc/testsuite/gcc.target/mips/msub-3.c
===================================================================
*** gcc/testsuite/gcc.target/mips/msub-3.c	(revision 127832)
--- gcc/testsuite/gcc.target/mips/msub-3.c	(working copy)
***************
*** 2,14 ****
  /* { dg-mips-options "-O2 -mips32 -mgp32" } */
  /* { dg-final { scan-assembler-times "\tmsub\t" 2 } } */
  
! long long
  f1 (int x, int y, long long z)
  {
    return z - (long long) y * x;
  }
  
! long long
  f2 (int x, int y, long long z)
  {
    long long t = (long long) x * y;
--- 2,14 ----
  /* { dg-mips-options "-O2 -mips32 -mgp32" } */
  /* { dg-final { scan-assembler-times "\tmsub\t" 2 } } */
  
! long long __attribute__ ((nomips16))
  f1 (int x, int y, long long z)
  {
    return z - (long long) y * x;
  }
  
! long long __attribute__ ((nomips16))
  f2 (int x, int y, long long z)
  {
    long long t = (long long) x * y;
Index: gcc/testsuite/gcc.target/mips/msubu-3.c
===================================================================
*** gcc/testsuite/gcc.target/mips/msubu-3.c	(revision 127832)
--- gcc/testsuite/gcc.target/mips/msubu-3.c	(working copy)
***************
*** 5,17 ****
  typedef unsigned int ui;
  typedef unsigned long long ull;
  
! ull
  f1 (ui x, ui y, ull z)
  {
    return z - (ull) y * x;
  }
  
! ull
  f2 (ui x, ui y, ull z)
  {
    ull t = (ull) x * y;
--- 5,17 ----
  typedef unsigned int ui;
  typedef unsigned long long ull;
  
! ull __attribute__ ((nomips16))
  f1 (ui x, ui y, ull z)
  {
    return z - (ull) y * x;
  }
  
! ull __attribute__ ((nomips16))
  f2 (ui x, ui y, ull z)
  {
    ull t = (ull) x * y;
Index: gcc/testsuite/gcc.target/mips/dspr2-MULT.c
===================================================================
*** gcc/testsuite/gcc.target/mips/dspr2-MULT.c	(revision 127832)
--- gcc/testsuite/gcc.target/mips/dspr2-MULT.c	(working copy)
*************** typedef long long a64;
*** 11,17 ****
  a64 a[4];
  int b[4], c[4];
  
! void test ()
  {
    a[0] = (a64) b[0] * c[0];
    a[1] = (a64) b[1] * c[1];
--- 11,17 ----
  a64 a[4];
  int b[4], c[4];
  
! void __attribute__ ((nomips16)) test ()
  {
    a[0] = (a64) b[0] * c[0];
    a[1] = (a64) b[1] * c[1];
Index: gcc/testsuite/gcc.target/mips/dspr2-MULTU.c
===================================================================
*** gcc/testsuite/gcc.target/mips/dspr2-MULTU.c	(revision 127832)
--- gcc/testsuite/gcc.target/mips/dspr2-MULTU.c	(working copy)
*************** typedef long long a64;
*** 11,17 ****
  a64 a[4];
  unsigned int b[4], c[4];
  
! void test ()
  {
    a[0] = (a64) b[0] * c[0];
    a[1] = (a64) b[1] * c[1];
--- 11,17 ----
  a64 a[4];
  unsigned int b[4], c[4];
  
! void __attribute__ ((nomips16)) test ()
  {
    a[0] = (a64) b[0] * c[0];
    a[1] = (a64) b[1] * c[1];

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: PATCH:  mips16/nomips16 function attributes, version N
  2007-08-27 20:14 PATCH: mips16/nomips16 function attributes, version N Sandra Loosemore
@ 2007-08-28  9:02 ` Richard Sandiford
  2007-08-29 19:56   ` Sandra Loosemore
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Sandiford @ 2007-08-28  9:02 UTC (permalink / raw)
  To: Sandra Loosemore; +Cc: GCC Patches, David Ung, Nigel Stephens, Mark Mitchell

Thanks for the updated patch.  This is mostly looking good to me,
but I think it needs another round.

Sandra Loosemore <sandra@codesourcery.com> writes:
> * I've implemented Richard's suggestion of taking out the call to 
> mips_set_mips16_mode in override_options, and just waiting until the 
> per-function hook has been invoked for the first time.

Hmm.  The idea was that -mips16 should just specify the default for
attributeless functions, so I was thinking that you should still keep
the code that clears the MIPS16 flag.

You'd need to change TARGET_CPU_CPP_BUILTINS to use mips_base_mips16
rather than TARGET_MIPS16.  You'd also need to keep the code that sets
targetm.have_tls in override_options, and use a new "base" variable to
make that the default.  But would much more need to change?

> + mflip-mips16
> + Target Report Var(TARGET_FLIP_MIPS16)
> + Switch on/off mips16 ASE on alternating functions for compiler testing

s/mips16/MIPS16/

> + /* Remember the ambient target flags, excluding mips16.  */
> + static GTY(()) int mips_base_target_flags;
> + /* The mips16 command-line target flags only.  */
> + static GTY(()) int mips_base_mips16;
> + /* Similar copies of option settings.  */
> + static int mips_base_schedule_insns; /* flag_schedule_insns */
> + static int mips_base_reorder_blocks_and_partition; /* flag_reorder... */
> + static int mips_base_align_loops; /* align_loops */
> + static int mips_base_align_jumps; /* align_jumps */
> + static int mips_base_align_functions; /* align_functions */
> + static GTY(()) int mips16_flipper;

This hasn't really been resolved IMO.  My understanding is that, after
optionally testing for compatibility, the PCH machinery continues to use
the _command-line_ value of target_flags (as one would hope).  Thus I don't
think we should GTY mips_base_target_flags, as that uses the _PCH_ value
of the flags.  mips_base_mips16 is the same.

If we're worried about fatal mismatches between the PCH flags
and the command-line flags, we should probably check for them in
TARGET_CHECK_PCH_TARGET_FLAGS.  That's an unrelated change though.

(mips16_flipper _does_ need to be GTYed; these comments about are
purely about the flag-related stuff.)

Please make mips_base_mips16 a bool and initialize it with:

  mips_base_mips16 = TARGET_MIPS16;

> ! /* Implement TARGET_FUNCTION_OK_FOR_SIBCALL.  */
>   
>   static bool
> ! mips_function_ok_for_sibcall (tree decl,
>   			      tree exp ATTRIBUTE_UNUSED)

Nit, but the parameters now fit on one line.

>   {
> !   /* Not if TARGET_SIBCALLS isn't set.  */
> !   if (!TARGET_SIBCALLS)
> !     return 0;
> ! 
> !   /* We can't do a sibcall if the called function is a MIPS16 function
> !      because there is no direct "jx" instruction equivalent to "jalx" to
> !      switch the ISA mode.  */
> !   if (decl && SYMBOL_REF_MIPS16_FUNC_P (XEXP (DECL_RTL (decl), 0)))
> !     return 0;
> ! 
> !   /* Otherwise OK.  */
> !   return 1;

Use false and true instead of 0 and 1.  I suppose the comment above
!TARGET_SIBCALLS isn't really useful.

> *************** function_arg (const CUMULATIVE_ARGS *cum
> *** 4225,4231 ****
>        stored as the mode.  */
>     if (mode == VOIDmode)
>       {
> !       if (TARGET_MIPS16 && cum->fp_code != 0)
>   	return gen_rtx_REG ((enum machine_mode) cum->fp_code, 0);
>   
>         else
> --- 4274,4281 ----
>        stored as the mode.  */
>     if (mode == VOIDmode)
>       {
> !       if ((TARGET_MIPS16 || mips_base_mips16)
> ! 	  && cum->fp_code != 0)
>   	return gen_rtx_REG ((enum machine_mode) cum->fp_code, 0);
>   
>         else

Is this still necessary, now that we've removed the special handling
for built-in functions?

> *************** mips_set_tune (const struct mips_cpu_inf
> *** 5031,5036 ****
> --- 5081,5312 ----
>       }
>   }
>   
> + /* mips_split_addresses is a half-way house between explicit
> +    relocations and the traditional assembler macros.  It can
> +    split absolute 32-bit symbolic constants into a high/lo_sum
> +    pair but uses macros for other sorts of access.
> +    
> +    Like explicit relocation support for REL targets, it relies
> +    on GNU extensions in the assembler and the linker.
> + 
> +    Although this code should work for -O0, it has traditionally
> +    been treated as an optimization.  */
> + 
> + static void
> + mips_init_split_addresses (void)
> + {
> +   if (!TARGET_MIPS16 && TARGET_SPLIT_ADDRESSES

Start the comment with a description of what the function does.  E.g.:

/* Initialize mips_split_addresses from the associated command-line settings.

   [...]  */

> + /* Set up the target-dependent global state so that it matches the
> +    current function's ISA mode.  */
> + static void

Nit: blank line after comment.

> +       /* Silently disable DSP extensions.  We already complained in
> + 	 override_options if both -mdsp and -mips16 were specified on the
> + 	 command line, so here we're assuming local mips16 function
> + 	 attribute takes precedence over command-line -mdsp.  */
> +       target_flags &= ~MASK_DSP;
> +       target_flags &= ~MASK_DSPR2;

I believe that error should be removed.  As per the above, we should now
be getting to the stage where -mips16 just specifies the default mode
for attributeless functions, so there's no reason now to complain about
that combination.  (The mask clearing is still needed.  I'm just talking
about the comment here.)

> +       /* Force no alignment of mips16 code.  */
> +       /* XXX would 32-bit alignment be an acceptable compromise?  */
> +       align_loops = align_jumps = align_functions = 1;

This doesn't quite implement what I said in the earlier review:

    http://article.gmane.org/gmane.comp.gcc.patches/145622/match=acceptable+compromise

namely:

> I think we should simply restore the defaults before the "if (mips16_p)"
> statement, like we do with the target flags, and move the overrides
> to the !mips16_p arm.

What I meant was: you should delete the "Force no alignment" code above
and move this code:

  /* Provide default values for align_* for 64-bit targets.  */
  if (TARGET_64BIT && !TARGET_MIPS16)
    {
      if (align_loops == 0)
	align_loops = 8;
      if (align_jumps == 0)
	align_jumps = 8;
      if (align_functions == 0)
	align_functions = 8;
    }

from override_options into the !mips16_p arm of mips_set_mips16_mode,
making it conditional on just TARGET_64BIT.

> +   /* (Re)initialise mips target internals for new ISA.  */

Coding conventions say this should be "ize".

> *************** mips_expand_builtin_bposge (enum mips_bu
> *** 12152,12157 ****
> --- 12332,12379 ----
>   				       const1_rtx, const0_rtx);
>   }
>   \f
> + /* Return true if we should force MIPS16 mode for the function named by
> +    the SYMBOL_REF SYMBOL, which belongs to DECL and has type TYPE.
> +    FIRST is true if this is the first time handling this decl.  */
> + static int
> + mips_use_mips16_mode_p (rtx symbol, tree decl, int first, tree type)

Blank line after comment.  Return a bool.

> + {
> +   int is_mips16 = mips_base_mips16;
> +   tree parent;
> + 
> +   if (mips_mips16_type_p (type))
> +     is_mips16 = 1;
> +   else if (mips_nomips16_type_p (type))
> +     is_mips16 = 0;
> +   else if (parent = decl_function_context (decl))
> +     /* Nested function should inherit MIPS16 setting from its parent.  */
> +     is_mips16 = SYMBOL_REF_MIPS16_FUNC_P (XEXP (DECL_RTL (parent), 0));

Remove the is_mips16 variable and just return true or false when you've
decided.  So:

  /* Function attributes take precedence.  */
  if (mips_mips16_type_p (type))
    return true;
  if (mips_nomips16_type_p (type))
    return false;

  /* A nested function should inherit the MIPS16 setting from its parent.  */
  parent = decl_function_context (decl);
  if (parent)
    return SYMBOL_REF_MIPS16_FUNC_P (XEXP (DECL_RTL (parent), 0));

  if (TARGET_FLIP_MIPS16
      && !DECL_BUILT_IN (decl)
      && !DECL_ARTIFICIAL (decl))
    {
      if (!first)
	/* Use the setting we picked first time around.  */
	return SYMBOL_REF_MIPS16_FUNC_P (symbol);

      mips16_flipper = !mips16_flipper;
      if (mips16_flipper)
	return !mips_base_mips16;
    }

  return mips_base_mips16;

> +   /* If there was an explicit -mno-mips16, then prevent MIPS16
> +      selection, even when an explicit attribute is given.  */
> +   if ((target_flags_explicit & MASK_MIPS16)
> +       && mips_base_mips16 == 0)
> +     is_mips16 = 0;

I thought we'd decided to drop this?  Sorry if I'm misremembering.
(FWIW, the comment in the original review was:

> This seems very counter-intuitive to me.  If we need a way of ignoring
> the attributes, let's add an option specifically for it.  I think that
> can be done as a follow-on patch, if there's demand.

)

Richard

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: PATCH:  mips16/nomips16 function attributes, version N
  2007-08-28  9:02 ` Richard Sandiford
@ 2007-08-29 19:56   ` Sandra Loosemore
  2007-08-29 23:04     ` Richard Sandiford
  0 siblings, 1 reply; 5+ messages in thread
From: Sandra Loosemore @ 2007-08-29 19:56 UTC (permalink / raw)
  To: GCC Patches, David Ung, Nigel Stephens, Mark Mitchell, richard

Richard Sandiford wrote:
> Thanks for the updated patch.  This is mostly looking good to me,
> but I think it needs another round.
> 
> Sandra Loosemore <sandra@codesourcery.com> writes:
>> * I've implemented Richard's suggestion of taking out the call to 
>> mips_set_mips16_mode in override_options, and just waiting until the 
>> per-function hook has been invoked for the first time.
> 
> Hmm.  The idea was that -mips16 should just specify the default for
> attributeless functions, so I was thinking that you should still keep
> the code that clears the MIPS16 flag.
> 
> You'd need to change TARGET_CPU_CPP_BUILTINS to use mips_base_mips16
> rather than TARGET_MIPS16.  You'd also need to keep the code that sets
> targetm.have_tls in override_options, and use a new "base" variable to
> make that the default.  But would much more need to change?

I don't understand why you think clearing the MIPS16 flag in 
override_options is The Right Thing To Do.  That means that the normal
first-time target initialization (the call to backend_init_target() from 
toplev.c) all happens in the "wrong" mode when -mips16 is passed. 
Trying to compensate for that by removing the first-time condition on 
the call to target_reinit() in mips_set_mips16_mode() causes a segfault, 
I think because it is happening before all the normal back end 
initialization is complete.  I haven't tried to poke at it in detail 
yet, because even if I can resolve that order-dependency issue, why 
initialize everything the wrong way to start with and then have to go 
back and immediately reinitialize everything the other way, when we can 
more easily just do it right the first time?

I'm now thinking that the correct thing to do is to restore this to the 
way it was before, with the explicit call to mips_set_mips16_mode in 
override_options, so that the order dependency of the first-time 
mips16-specific initialization actions is made explicit.  It's awfully 
hard to tell from reading the code just when that hook function is going 
to be invoked for the first time....

I didn't have any issues with your other comments -- thanks for your 
usual thorough review.  :-)

-Sandra

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: PATCH:  mips16/nomips16 function attributes, version N
  2007-08-29 19:56   ` Sandra Loosemore
@ 2007-08-29 23:04     ` Richard Sandiford
  2007-08-30  2:02       ` Sandra Loosemore
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Sandiford @ 2007-08-29 23:04 UTC (permalink / raw)
  To: Sandra Loosemore; +Cc: GCC Patches, David Ung, Nigel Stephens, Mark Mitchell

Sandra Loosemore <sandra@codesourcery.com> writes:
> Richard Sandiford wrote:
>>> * I've implemented Richard's suggestion of taking out the call to 
>>> mips_set_mips16_mode in override_options, and just waiting until the 
>>> per-function hook has been invoked for the first time.
>> 
>> Hmm.  The idea was that -mips16 should just specify the default for
>> attributeless functions, so I was thinking that you should still keep
>> the code that clears the MIPS16 flag.
>> 
>> You'd need to change TARGET_CPU_CPP_BUILTINS to use mips_base_mips16
>> rather than TARGET_MIPS16.  You'd also need to keep the code that sets
>> targetm.have_tls in override_options, and use a new "base" variable to
>> make that the default.  But would much more need to change?
>
> I don't understand why you think clearing the MIPS16 flag in 
> override_options is The Right Thing To Do.  That means that the normal
> first-time target initialization (the call to backend_init_target() from 
> toplev.c) all happens in the "wrong" mode when -mips16 is passed. 
> Trying to compensate for that by removing the first-time condition on 
> the call to target_reinit() in mips_set_mips16_mode() causes a segfault, 
> I think because it is happening before all the normal back end 
> initialization is complete.  I haven't tried to poke at it in detail 
> yet, because even if I can resolve that order-dependency issue, why 
> initialize everything the wrong way to start with and then have to go 
> back and immediately reinitialize everything the other way, when we can 
> more easily just do it right the first time?

I felt it was more robust.  I just liked the mental model that -mips16
specified the default mode for every function without an explicit mode
attribute (including compiler-generated functions).  I thought we were
more likely to get that behaviour if we start the compiler in the same
way all the time, and only switch to MIPS16 mode when the hook is first
called.  It seemed likely that, if we _start_ with everything in MIPS16
mode, we're likely to pull in certain MIPS16 properties from that fact
alone, rather than pulling them in from the mode-switching hook.

I suppose I was hoping that, one day, the compiler would be in a
"mode-agnostic" state when outside the top-level push_cfun/pop_cfun
pair, except for explicit exceptions like TARGET_CPU_CPP_BUILTINS.
However, that would mean deferring backend_init until the first call
to your new hook, which would probably be even more work than what
you've done so far.  Even then, the compiler would not be in a
mode-agnostic state between a top-level pop_cfun and the next
push_cfun.  I suppose that day will only come "when" (= if) all
mode-specific data is in an easily-switchable struct.

As it is, the comments you made on internal IRC suggest that this
reinitialisation is very expensive, so I suppose it's not something
we want to foist on normal -mips16 users for the sake of an internal
mental model.  So...

> I'm now thinking that the correct thing to do is to restore this to the 
> way it was before, with the explicit call to mips_set_mips16_mode in 
> override_options, so that the order dependency of the first-time 
> mips16-specific initialization actions is made explicit.  It's awfully 
> hard to tell from reading the code just when that hook function is going 
> to be invoked for the first time....

...OK.  Thanks for trying it out, and sorry for the run-aroud.

From the segfault you describe, it sounds like we currently assume that
calling this hook is a no-op until the first time we pass a function
with attributes.  (Or at least, it sounds like that rule would avoid the
situations where things go wrong.)  If so, I think we should mention
that in the tm.texi documentation.

Richard

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: PATCH:  mips16/nomips16 function attributes, version N
  2007-08-29 23:04     ` Richard Sandiford
@ 2007-08-30  2:02       ` Sandra Loosemore
  0 siblings, 0 replies; 5+ messages in thread
From: Sandra Loosemore @ 2007-08-30  2:02 UTC (permalink / raw)
  To: GCC Patches, David Ung, Nigel Stephens, Mark Mitchell, richard

Richard Sandiford wrote:

> I suppose I was hoping that, one day, the compiler would be in a
> "mode-agnostic" state when outside the top-level push_cfun/pop_cfun
> pair, except for explicit exceptions like TARGET_CPU_CPP_BUILTINS.
> However, that would mean deferring backend_init until the first call
> to your new hook, which would probably be even more work than what
> you've done so far.  Even then, the compiler would not be in a
> mode-agnostic state between a top-level pop_cfun and the next
> push_cfun.  I suppose that day will only come "when" (= if) all
> mode-specific data is in an easily-switchable struct.

Right.  In a perfect world, what we'd do here is just have two objects 
encapsulating all the back end state for each mode, and all we'd have to 
swap is a pointer.  But reverse-engineering that onto the existing code 
would be very nasty, and likely introduce many bugs.

> As it is, the comments you made on internal IRC suggest that this
> reinitialisation is very expensive, so I suppose it's not something
> we want to foist on normal -mips16 users for the sake of an internal
> mental model.

To give a rough idea of the magnitude of the performance hit, my 
previous version of these patches that invoked the reset hook only once 
per function ran about 20% slower in -mflip-mips16 mode than without, 
judging just by time stamps on log files from running the gcc testsuite. 
  With the new version and the hook being invoked every time cfun is 
set, that's gone up to a factor of 3.  Of course -mflip-mips16 is 
designed to be pathological; in practice, use of these attributes is 
going to be exceptional, and we should optimize for the common case instead.

As I noted in my comments about the other part of the patch, I think 
some of the places where cfun is being set to NULL are really not 
necessary (they seem to be signalling that a processing phase is done 
rather than a real function context switch), and getting rid of those 
places would speed things up.  Again, it's only a concern for things 
that aren't using the default mode.

> From the segfault you describe, it sounds like we currently assume that
> calling this hook is a no-op until the first time we pass a function
> with attributes.  (Or at least, it sounds like that rule would avoid the
> situations where things go wrong.)  If so, I think we should mention
> that in the tm.texi documentation.

Yes, I am worried about this too.  I already had to add a hack to 
prevent the hook from being invoked recursively; maybe I need to 
generalize that with something to prevent it from being invoked before 
initial initialization :-) is complete, and/or put an assertion in 
target_reinit ().  I'm going to poke around at this once I've tested the 
other changes you suggested.

-Sandra

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2007-08-29 23:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-08-27 20:14 PATCH: mips16/nomips16 function attributes, version N Sandra Loosemore
2007-08-28  9:02 ` Richard Sandiford
2007-08-29 19:56   ` Sandra Loosemore
2007-08-29 23:04     ` Richard Sandiford
2007-08-30  2:02       ` Sandra Loosemore

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).