public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Add preferred_for_{size,speed} attributes
@ 2014-10-17 14:47 Richard Sandiford
  2014-10-17 14:48 ` [PATCH 1/5] Add recog_constrain_insn Richard Sandiford
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Richard Sandiford @ 2014-10-17 14:47 UTC (permalink / raw)
  To: gcc-patches; +Cc: vmakarov, ubizjak, markus, Ganesh.Gopalasubramanian

This patch implements the approach I suggested in:

    https://gcc.gnu.org/ml/gcc-patches/2014-10/msg00371.html

for fixing PR61360.  To recap, the problem is with the use of "enabled" in
the i386.md pattern:

(define_insn "*float<SWI48:mode><MODEF:mode>2_sse"
  [(set (match_operand:MODEF 0 "register_operand" "=f,x,x")
	(float:MODEF
	  (match_operand:SWI48 1 "nonimmediate_operand" "m,r,m")))]
  "SSE_FLOAT_MODE_P (<MODEF:MODE>mode) && TARGET_SSE_MATH"
  "@
   fild%Z1\t%1
   %vcvtsi2<MODEF:ssemodesuffix><SWI48:rex64suffix>\t{%1, %d0|%d0, %1}
   %vcvtsi2<MODEF:ssemodesuffix><SWI48:rex64suffix>\t{%1, %d0|%d0, %1}"
  [(set_attr "type" "fmov,sseicvt,sseicvt")
   (set_attr "prefix" "orig,maybe_vex,maybe_vex")
   (set_attr "mode" "<MODEF:MODE>")
   (set (attr "prefix_rex")
     (if_then_else
       (and (eq_attr "prefix" "maybe_vex")
	    (match_test "<SWI48:MODE>mode == DImode"))
       (const_string "1")
       (const_string "*")))
   (set_attr "unit" "i387,*,*")
   (set_attr "athlon_decode" "*,double,direct")
   (set_attr "amdfam10_decode" "*,vector,double")
   (set_attr "bdver1_decode" "*,double,direct")
   (set_attr "fp_int_src" "true")
   (set (attr "enabled")
     (cond [(eq_attr "alternative" "0")
              (symbol_ref "TARGET_MIX_SSE_I387
                           && X87_ENABLE_FLOAT (<MODEF:MODE>mode,
                                                <SWI48:MODE>mode)")
            (eq_attr "alternative" "1")
              /* ??? For sched1 we need constrain_operands to be able to
                 select an alternative.  Leave this enabled before RA.  */
              (symbol_ref "TARGET_INTER_UNIT_CONVERSIONS
                           || optimize_function_for_size_p (cfun)
                           || !(reload_completed
                                || reload_in_progress
                                || lra_in_progress)")
           ]
           (symbol_ref "true")))
   ])

The attribute was only really supposed to test properties of the currently-
selected target.  It wasn't supposed to test function-specific size/speed
properties like the above pattern does.

So the idea was instead to add two new attributes that say whether
an alternative should be used when optimising for speed or size.
These attributes would just be strong optimisation hints; they wouldn't
be correctness properties in the way that "enabled" is.  There are some
cases where we could end up with a size-only alternative in code
optimised for speed, or vice versa, but the idea would be to reduce
them as far as possible.

The main advantage of this approach is that we can take block-level
size/speed choices into account, rather than just looking at the
function-level choice.

Series tested on x86_64-linux-gnu.

Thanks,
Richard

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

* [PATCH 1/5] Add recog_constrain_insn
  2014-10-17 14:47 [PATCH 0/5] Add preferred_for_{size,speed} attributes Richard Sandiford
@ 2014-10-17 14:48 ` Richard Sandiford
  2014-10-21 14:56   ` Vladimir Makarov
  2014-10-21 15:45   ` Jeff Law
  2014-10-17 14:51 ` [PATCH 2/5] Add preferred_for_{size,speed} attributes Richard Sandiford
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 17+ messages in thread
From: Richard Sandiford @ 2014-10-17 14:48 UTC (permalink / raw)
  To: gcc-patches; +Cc: vmakarov, ubizjak, markus, Ganesh.Gopalasubramanian

This patch just adds a new utility function called recog_constrain_insn,
to go alongside the existing recog_constrain_insn_cached.

Note that the extract_insn in lra.c wasn't used when checking is disabled.
The function just moved on to the next instruction straight away.

Richard


gcc/
	* recog.h (extract_constrain_insn): Declare.
	* recog.c (extract_constrain_insn): New function.
	* lra.c (check_rtl): Use it.
	* postreload.c (reload_cse_simplify_operands): Likewise.
	* reg-stack.c (check_asm_stack_operands): Likewise.
	(subst_asm_stack_regs): Likewise.
	* regcprop.c (copyprop_hardreg_forward_1): Likewise.
	* regrename.c (build_def_use): Likewise.
	* sel-sched.c (get_reg_class): Likewise.
	* config/arm/arm.c (note_invalid_constants): Likewise.
	* config/s390/predicates.md (execute_operation): Likewise.

Index: gcc/recog.h
===================================================================
--- gcc/recog.h	2014-09-18 11:40:31.223690858 +0100
+++ gcc/recog.h	2014-10-17 15:44:50.219398486 +0100
@@ -134,6 +134,7 @@ extern void add_clobbers (rtx, int);
 extern int added_clobbers_hard_reg_p (int);
 extern void insn_extract (rtx_insn *);
 extern void extract_insn (rtx_insn *);
+extern void extract_constrain_insn (rtx_insn *insn);
 extern void extract_constrain_insn_cached (rtx_insn *);
 extern void extract_insn_cached (rtx_insn *);
 extern void preprocess_constraints (int, int, const char **,
Index: gcc/recog.c
===================================================================
--- gcc/recog.c	2014-09-22 08:36:23.889794255 +0100
+++ gcc/recog.c	2014-10-17 15:44:50.219398486 +0100
@@ -2110,6 +2110,17 @@ extract_insn_cached (rtx_insn *insn)
   recog_data.insn = insn;
 }
 
+/* Do uncached extract_insn, constrain_operands and complain about failures.
+   This should be used when extracting a pre-existing constrained instruction
+   if the caller wants to know which alternative was chosen.  */
+void
+extract_constrain_insn (rtx_insn *insn)
+{
+  extract_insn (insn);
+  if (!constrain_operands (reload_completed))
+    fatal_insn_not_found (insn);
+}
+
 /* Do cached extract_insn, constrain_operands and complain about failures.
    Used by insn_attrtab.  */
 void
Index: gcc/lra.c
===================================================================
--- gcc/lra.c	2014-09-26 16:05:57.868394574 +0100
+++ gcc/lra.c	2014-10-17 15:44:50.219398486 +0100
@@ -1919,8 +1919,9 @@ check_rtl (bool final_p)
       {
 	if (final_p)
 	  {
-	    extract_insn (insn);
-	    lra_assert (constrain_operands (1));
+#ifdef ENABLED_CHECKING
+	    extract_constrain_insn (insn);
+#endif
 	    continue;
 	  }
 	/* LRA code is based on assumption that all addresses can be
Index: gcc/postreload.c
===================================================================
--- gcc/postreload.c	2014-08-26 12:09:02.182959856 +0100
+++ gcc/postreload.c	2014-10-17 15:44:50.219398486 +0100
@@ -401,15 +401,11 @@ reload_cse_simplify_operands (rtx_insn *
   /* Array of alternatives, sorted in order of decreasing desirability.  */
   int *alternative_order;
 
-  extract_insn (insn);
+  extract_constrain_insn (insn);
 
   if (recog_data.n_alternatives == 0 || recog_data.n_operands == 0)
     return 0;
 
-  /* Figure out which alternative currently matches.  */
-  if (! constrain_operands (1))
-    fatal_insn_not_found (insn);
-
   alternative_reject = XALLOCAVEC (int, recog_data.n_alternatives);
   alternative_nregs = XALLOCAVEC (int, recog_data.n_alternatives);
   alternative_order = XALLOCAVEC (int, recog_data.n_alternatives);
Index: gcc/reg-stack.c
===================================================================
--- gcc/reg-stack.c	2014-09-18 11:40:31.307689884 +0100
+++ gcc/reg-stack.c	2014-10-17 15:44:50.219398486 +0100
@@ -469,8 +469,7 @@ check_asm_stack_operands (rtx_insn *insn
 
   /* Find out what the constraints require.  If no constraint
      alternative matches, this asm is malformed.  */
-  extract_insn (insn);
-  constrain_operands (1);
+  extract_constrain_insn (insn);
 
   preprocess_constraints (insn);
 
@@ -2016,8 +2015,7 @@ subst_asm_stack_regs (rtx_insn *insn, st
   /* Find out what the constraints required.  If no constraint
      alternative matches, that is a compiler bug: we should have caught
      such an insn in check_asm_stack_operands.  */
-  extract_insn (insn);
-  constrain_operands (1);
+  extract_constrain_insn (insn);
 
   preprocess_constraints (insn);
   const operand_alternative *op_alt = which_op_alt ();
Index: gcc/regcprop.c
===================================================================
--- gcc/regcprop.c	2014-10-13 08:02:41.225135081 +0100
+++ gcc/regcprop.c	2014-10-17 15:44:50.227398391 +0100
@@ -762,9 +762,7 @@ copyprop_hardreg_forward_1 (basic_block
 	}
 
       set = single_set (insn);
-      extract_insn (insn);
-      if (! constrain_operands (1))
-	fatal_insn_not_found (insn);
+      extract_constrain_insn (insn);
       preprocess_constraints (insn);
       const operand_alternative *op_alt = which_op_alt ();
       n_ops = recog_data.n_operands;
@@ -865,9 +863,7 @@ copyprop_hardreg_forward_1 (basic_block
 		}
 	      /* We need to re-extract as validate_change clobbers
 		 recog_data.  */
-	      extract_insn (insn);
-	      if (! constrain_operands (1))
-		fatal_insn_not_found (insn);
+	      extract_constrain_insn (insn);
 	      preprocess_constraints (insn);
 	    }
 
@@ -893,9 +889,7 @@ copyprop_hardreg_forward_1 (basic_block
 		    }
 		  /* We need to re-extract as validate_change clobbers
 		     recog_data.  */
-		  extract_insn (insn);
-		  if (! constrain_operands (1))
-		    fatal_insn_not_found (insn);
+		  extract_constrain_insn (insn);
 		  preprocess_constraints (insn);
 		}
 	    }
Index: gcc/regrename.c
===================================================================
--- gcc/regrename.c	2014-08-26 12:08:57.679011819 +0100
+++ gcc/regrename.c	2014-10-17 15:44:50.227398391 +0100
@@ -1564,9 +1564,7 @@ build_def_use (basic_block bb)
 	     to be marked unrenamable or even cause us to abort the entire
 	     basic block.  */
 
-	  extract_insn (insn);
-	  if (! constrain_operands (1))
-	    fatal_insn_not_found (insn);
+	  extract_constrain_insn (insn);
 	  preprocess_constraints (insn);
 	  const operand_alternative *op_alt = which_op_alt ();
 	  n_ops = recog_data.n_operands;
Index: gcc/sel-sched.c
===================================================================
--- gcc/sel-sched.c	2014-09-18 11:40:31.163691553 +0100
+++ gcc/sel-sched.c	2014-10-17 15:44:50.227398391 +0100
@@ -994,9 +994,7 @@ get_reg_class (rtx_insn *insn)
 {
   int i, n_ops;
 
-  extract_insn (insn);
-  if (! constrain_operands (1))
-    fatal_insn_not_found (insn);
+  extract_constrain_insn (insn);
   preprocess_constraints (insn);
   n_ops = recog_data.n_operands;
 
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c	2014-09-22 08:36:24.541786033 +0100
+++ gcc/config/arm/arm.c	2014-10-17 15:44:50.215398533 +0100
@@ -17022,10 +17022,7 @@ note_invalid_constants (rtx_insn *insn,
 {
   int opno;
 
-  extract_insn (insn);
-
-  if (!constrain_operands (1))
-    fatal_insn_not_found (insn);
+  extract_constrain_insn (insn);
 
   if (recog_data.n_alternatives == 0)
     return;
Index: gcc/config/s390/predicates.md
===================================================================
--- gcc/config/s390/predicates.md	2014-09-12 08:26:02.847292053 +0100
+++ gcc/config/s390/predicates.md	2014-10-17 15:44:50.219398486 +0100
@@ -406,8 +406,7 @@ (define_special_predicate "execute_opera
   if (icode < 0)
     return false;
 
-  extract_insn (insn);
-  constrain_operands (1);
+  extract_constrain_insn (insn);
 
   return which_alternative >= 0;
 })

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

* [PATCH 2/5] Add preferred_for_{size,speed} attributes
  2014-10-17 14:47 [PATCH 0/5] Add preferred_for_{size,speed} attributes Richard Sandiford
  2014-10-17 14:48 ` [PATCH 1/5] Add recog_constrain_insn Richard Sandiford
@ 2014-10-17 14:51 ` Richard Sandiford
  2014-10-21 14:57   ` Vladimir Makarov
                     ` (2 more replies)
  2014-10-17 14:52 ` [PATCH 3/5] Pass an alternative_mask to constrain_operands Richard Sandiford
                   ` (2 subsequent siblings)
  4 siblings, 3 replies; 17+ messages in thread
From: Richard Sandiford @ 2014-10-17 14:51 UTC (permalink / raw)
  To: gcc-patches; +Cc: vmakarov, ubizjak, markus, Ganesh.Gopalasubramanian

This is the main patch, to add new preferred_for_size and
preferred_for_speed attributes that can be used to selectively disable
alternatives when optimising for size or speed.  As explained in the
docs, the new attributes are just optimisation hints and it is possible
that "size-only" alternatives will sometimes end up in a block that's
optimised for speed, or vice versa.

The patch deals with code that directly accesses the enabled_attributes
mask and that ought to take size/speed choices into account.  The next
patch deals with indirect uses.  Note that I'm not making reload support
these attributes for hopefully obvious reasons :-)

Richard


gcc/
	* doc/md.texi: Document "preferred_for_size" and "preferred_for_speed"
	attributes.
	* genattr.c (main): Handle "preferred_for_size" and
	"preferred_for_speed" in the same way as "enabled".
	* recog.h (bool_attr): New enum.
	(target_recog): Replace x_enabled_alternatives with x_bool_attr_masks.
	(get_preferred_alternatives, check_bool_attrs): Declare.
	* recog.c (have_bool_attr, get_bool_attr, get_bool_attr_mask_uncached)
	(get_bool_attr_mask, get_preferred_alternatives, check_bool_attrs):
	New functions.
	(get_enabled_alternatives): Use get_bool_attr_mask.
	* ira-costs.c (record_reg_classes): Use get_preferred_alternatives
	instead of recog_data.enabled_alternatives.
	* ira.c (ira_setup_alts): Likewise.
	* postreload.c (reload_cse_simplify_operands): Likewise.
	* config/i386/i386.c (ix86_legitimate_combined_insn): Likewise.
	* ira-lives.c (preferred_alternatives): New variable.
	(process_bb_node_lives): Set it.
	(check_and_make_def_conflict, make_early_clobber_and_input_conflicts)
	(single_reg_class, ira_implicitly_set_insn_hard_regs): Use it instead
	of recog_data.enabled_alternatives.
	* lra-int.h (lra_insn_recog_data): Replace enabled_alternatives
	to preferred_alternatives.
	* lra-constraints.c (process_alt_operands): Update accordingly.
	* lra.c (lra_set_insn_recog_data): Likewise.
	(lra_update_insn_recog_data): Assert check_bool_attrs.

Index: gcc/doc/md.texi
===================================================================
--- gcc/doc/md.texi	2014-10-07 13:12:12.227445290 +0100
+++ gcc/doc/md.texi	2014-10-17 15:47:34.349453560 +0100
@@ -1080,7 +1080,7 @@ the addressing register.
 * Class Preferences::   Constraints guide which hard register to put things in.
 * Modifiers::           More precise control over effects of constraints.
 * Machine Constraints:: Existing constraints for some particular machines.
-* Disable Insn Alternatives:: Disable insn alternatives using the @code{enabled} attribute.
+* Disable Insn Alternatives:: Disable insn alternatives using attributes.
 * Define Constraints::  How to define machine-specific constraints.
 * C Constraint Interface:: How to test constraints from C code.
 @end menu
@@ -4006,42 +4006,49 @@ Unsigned constant valid for BccUI instru
 @subsection Disable insn alternatives using the @code{enabled} attribute
 @cindex enabled
 
-The @code{enabled} insn attribute may be used to disable insn
-alternatives that are not available for the current subtarget.
-This is useful when adding new instructions to an existing pattern
-which are only available for certain cpu architecture levels as
-specified with the @code{-march=} option.
-
-If an insn alternative is disabled, then it will never be used.  The
-compiler treats the constraints for the disabled alternative as
-unsatisfiable.
+There are three insn attributes that may be used to selectively disable
+instruction alternatives:
 
-In order to make use of the @code{enabled} attribute a back end has to add
-in the machine description files:
+@table @code
+@item enabled
+Says whether an alternative is available on the current subtarget.
 
-@enumerate
-@item
-A definition of the @code{enabled} insn attribute.  The attribute is
-defined as usual using the @code{define_attr} command.  This
-definition should be based on other insn attributes and/or target flags.
-The attribute must be a static property of the subtarget; that is, it
-must not depend on the current operands or any other dynamic context
-(for example, the location of the insn within the body of a loop).
-
-The @code{enabled} attribute is a numeric attribute and should evaluate to
-@code{(const_int 1)} for an enabled alternative and to
-@code{(const_int 0)} otherwise.
-@item
-A definition of another insn attribute used to describe for what
-reason an insn alternative might be available or
-not.  E.g. @code{cpu_facility} as in the example below.
-@item
-An assignment for the second attribute to each insn definition
-combining instructions which are not all available under the same
-circumstances.  (Note: It obviously only makes sense for definitions
-with more than one alternative.  Otherwise the insn pattern should be
-disabled or enabled using the insn condition.)
-@end enumerate
+@item preferred_for_size
+Says whether an enabled alternative should be used in code that is
+optimized for size.
+
+@item preferred_for_speed
+Says whether an enabled alternative should be used in code that is
+optimized for speed.
+@end table
+
+All these attributes should use @code{(const_int 1)} to allow an alternative
+or @code{(const_int 0)} to disallow it.  The attributes must be a static
+property of the subtarget; they cannot for example depend on the
+current operands, on the current optimization level, on the location
+of the insn within the body of a loop, on whether register allocation
+has finished, or on the current compiler pass.
+
+The @code{enabled} attribute is a correctness property.  It tells GCC to act
+as though the disabled alternatives were never defined in the first place.
+This is useful when adding new instructions to an existing pattern in
+cases where the new instructions are only available for certain cpu
+architecture levels (typically mapped to the @code{-march=} command-line
+option).
+
+In contrast, the @code{preferred_for_size} and @code{preferred_for_speed}
+attributes are strong optimization hints rather than correctness properties.
+@code{preferred_for_size} tells GCC which alternatives to consider when
+adding or modifying an instruction that GCC wants to optimize for size.
+@code{preferred_for_speed} does the same thing for speed.  Note that things
+like code motion can lead to cases where code optimized for size uses
+alternatives that are not preferred for size, and similarly for speed.
+
+Although @code{define_insn}s can in principle specify the @code{enabled}
+attribute directly, it is often clearer to have subsiduary attributes
+for each architectural feature of interest.  The @code{define_insn}s
+can then use these subsiduary attributes to say which alternatives
+require which features.  The example below does this for @code{cpu_facility}.
 
 E.g. the following two patterns could easily be merged using the @code{enabled}
 attribute:
Index: gcc/genattr.c
===================================================================
--- gcc/genattr.c	2014-09-18 11:40:31.195691182 +0100
+++ gcc/genattr.c	2014-10-17 15:47:34.353453512 +0100
@@ -338,7 +338,9 @@ main (int argc, char **argv)
     }
 
   /* Special-purpose attributes should be tested with if, not #ifdef.  */
-  const char * const special_attrs[] = { "length", "enabled", 0 };
+  const char * const special_attrs[] = { "length", "enabled",
+					 "preferred_for_size",
+					 "preferred_for_speed", 0 };
   for (const char * const *p = special_attrs; *p; p++)
     {
       printf ("#ifndef HAVE_ATTR_%s\n"
@@ -355,9 +357,15 @@ main (int argc, char **argv)
 	"#define insn_current_length hook_int_rtx_insn_unreachable\n"
 	"#include \"insn-addr.h\"\n"
 	"#endif\n"
-	"#if !HAVE_ATTR_enabled\n"
 	"extern int hook_int_rtx_1 (rtx);\n"
+	"#if !HAVE_ATTR_enabled\n"
 	"#define get_attr_enabled hook_int_rtx_1\n"
+	"#endif\n"
+	"#if !HAVE_ATTR_preferred_for_size\n"
+	"#define get_attr_preferred_for_size hook_int_rtx_1\n"
+	"#endif\n"
+	"#if !HAVE_ATTR_preferred_for_speed\n"
+	"#define get_attr_preferred_for_speed hook_int_rtx_1\n"
 	"#endif\n");
 
   /* Output flag masks for use by reorg.
Index: gcc/recog.h
===================================================================
--- gcc/recog.h	2014-10-17 15:44:50.219398486 +0100
+++ gcc/recog.h	2014-10-17 15:47:34.357453465 +0100
@@ -389,10 +389,19 @@ struct insn_data_d
 #ifndef GENERATOR_FILE
 #include "insn-codes.h"
 
+/* An enum of boolean attributes that may only depend on the current
+   subtarget, not on things like operands or compiler phase.  */
+enum bool_attr {
+  BA_ENABLED,
+  BA_PREFERRED_FOR_SPEED,
+  BA_PREFERRED_FOR_SIZE,
+  BA_LAST = BA_PREFERRED_FOR_SIZE
+};
+
 /* Target-dependent globals.  */
 struct target_recog {
   bool x_initialized;
-  alternative_mask x_enabled_alternatives[LAST_INSN_CODE];
+  alternative_mask x_bool_attr_masks[LAST_INSN_CODE][BA_LAST + 1];
   operand_alternative *x_op_alt[LAST_INSN_CODE];
 };
 
@@ -404,6 +413,8 @@ #define this_target_recog (&default_targ
 #endif
 
 alternative_mask get_enabled_alternatives (rtx_insn *);
+alternative_mask get_preferred_alternatives (rtx_insn *);
+bool check_bool_attrs (rtx_insn *);
 
 void recog_init ();
 #endif
Index: gcc/recog.c
===================================================================
--- gcc/recog.c	2014-10-17 15:44:50.219398486 +0100
+++ gcc/recog.c	2014-10-17 15:47:34.357453465 +0100
@@ -2055,25 +2055,46 @@ mode_dependent_address_p (rtx addr, addr
   return targetm.mode_dependent_address_p (addr, addrspace);
 }
 \f
-/* Return the mask of operand alternatives that are allowed for INSN.
-   This mask depends only on INSN and on the current target; it does not
-   depend on things like the values of operands.  */
+/* Return true if boolean attribute ATTR is supported.  */
 
-alternative_mask
-get_enabled_alternatives (rtx_insn *insn)
+static bool
+have_bool_attr (bool_attr attr)
 {
-  /* Quick exit for asms and for targets that don't use the "enabled"
-     attribute.  */
-  int code = INSN_CODE (insn);
-  if (code < 0 || !HAVE_ATTR_enabled)
-    return ALL_ALTERNATIVES;
+  switch (attr)
+    {
+    case BA_ENABLED:
+      return HAVE_ATTR_enabled;
+    case BA_PREFERRED_FOR_SIZE:
+      return HAVE_ATTR_enabled || HAVE_ATTR_preferred_for_size;
+    case BA_PREFERRED_FOR_SPEED:
+      return HAVE_ATTR_enabled || HAVE_ATTR_preferred_for_speed;
+    }
+  gcc_unreachable ();
+}
 
-  /* Calling get_attr_enabled can be expensive, so cache the mask
-     for speed.  */
-  if (this_target_recog->x_enabled_alternatives[code])
-    return this_target_recog->x_enabled_alternatives[code];
+/* Return the value of ATTR for instruction INSN.  */
 
-  /* Temporarily install enough information for get_attr_enabled to assume
+static bool
+get_bool_attr (rtx_insn *insn, bool_attr attr)
+{
+  switch (attr)
+    {
+    case BA_ENABLED:
+      return get_attr_enabled (insn);
+    case BA_PREFERRED_FOR_SIZE:
+      return get_attr_enabled (insn) && get_attr_preferred_for_size (insn);
+    case BA_PREFERRED_FOR_SPEED:
+      return get_attr_enabled (insn) && get_attr_preferred_for_speed (insn);
+    }
+  gcc_unreachable ();
+}
+
+/* Like get_bool_attr_mask, but don't use the cache.  */
+
+static alternative_mask
+get_bool_attr_mask_uncached (rtx_insn *insn, bool_attr attr)
+{
+  /* Temporarily install enough information for get_attr_<foo> to assume
      that the insn operands are already cached.  As above, the attribute
      mustn't depend on the values of operands, so we don't provide their
      real values here.  */
@@ -2081,20 +2102,81 @@ get_enabled_alternatives (rtx_insn *insn
   int old_alternative = which_alternative;
 
   recog_data.insn = insn;
-  alternative_mask enabled = ALL_ALTERNATIVES;
-  int n_alternatives = insn_data[code].n_alternatives;
+  alternative_mask mask = ALL_ALTERNATIVES;
+  int n_alternatives = insn_data[INSN_CODE (insn)].n_alternatives;
   for (int i = 0; i < n_alternatives; i++)
     {
       which_alternative = i;
-      if (!get_attr_enabled (insn))
-	enabled &= ~ALTERNATIVE_BIT (i);
+      if (!get_bool_attr (insn, attr))
+	mask &= ~ALTERNATIVE_BIT (i);
     }
 
   recog_data.insn = old_insn;
   which_alternative = old_alternative;
+  return mask;
+}
+
+/* Return the mask of operand alternatives that are allowed for INSN
+   by boolean attribute ATTR.  This mask depends only on INSN and on
+   the current target; it does not depend on things like the values of
+   operands.  */
+
+static alternative_mask
+get_bool_attr_mask (rtx_insn *insn, bool_attr attr)
+{
+  /* Quick exit for asms and for targets that don't use these attributes.  */
+  int code = INSN_CODE (insn);
+  if (code < 0 || !have_bool_attr (attr))
+    return ALL_ALTERNATIVES;
 
-  this_target_recog->x_enabled_alternatives[code] = enabled;
-  return enabled;
+  /* Calling get_attr_<foo> can be expensive, so cache the mask
+     for speed.  */
+  if (!this_target_recog->x_bool_attr_masks[code][attr])
+    this_target_recog->x_bool_attr_masks[code][attr]
+      = get_bool_attr_mask_uncached (insn, attr);
+  return this_target_recog->x_bool_attr_masks[code][attr];
+}
+
+/* Return the set of alternatives of INSN that are allowed by the current
+   target.  */
+
+alternative_mask
+get_enabled_alternatives (rtx_insn *insn)
+{
+  return get_bool_attr_mask (insn, BA_ENABLED);
+}
+
+/* Return the set of alternatives of INSN that are allowed by the current
+   target and are preferred for the current size/speed optimization
+   choice.  */
+
+alternative_mask
+get_preferred_alternatives (rtx_insn *insn)
+{
+  if (optimize_bb_for_speed_p (BLOCK_FOR_INSN (insn)))
+    return get_bool_attr_mask (insn, BA_PREFERRED_FOR_SPEED);
+  else
+    return get_bool_attr_mask (insn, BA_PREFERRED_FOR_SIZE);
+}
+
+/* Assert that the cached boolean attributes for INSN are still accurate.
+   The backend is required to define these attributes in a way that only
+   depends on the current target (rather than operands, compiler phase,
+   etc.).  */
+
+bool
+check_bool_attrs (rtx_insn *insn)
+{
+  int code = INSN_CODE (insn);
+  if (code >= 0)
+    for (int i = 0; i <= BA_LAST; ++i)
+      {
+	enum bool_attr attr = (enum bool_attr) i;
+	if (this_target_recog->x_bool_attr_masks[code][attr])
+	  gcc_assert (this_target_recog->x_bool_attr_masks[code][attr]
+		      == get_bool_attr_mask_uncached (insn, attr));
+      }
+  return true;
 }
 
 /* Like extract_insn, but save insn extracted and don't extract again, when
@@ -4043,8 +4125,8 @@ recog_init ()
       this_target_recog->x_initialized = true;
       return;
     }
-  memset (this_target_recog->x_enabled_alternatives, 0,
-	  sizeof (this_target_recog->x_enabled_alternatives));
+  memset (this_target_recog->x_bool_attr_masks, 0,
+	  sizeof (this_target_recog->x_bool_attr_masks));
   for (int i = 0; i < LAST_INSN_CODE; ++i)
     if (this_target_recog->x_op_alt[i])
       {
Index: gcc/ira-costs.c
===================================================================
--- gcc/ira-costs.c	2014-10-03 11:09:08.000000000 +0100
+++ gcc/ira-costs.c	2014-10-17 15:47:34.353453512 +0100
@@ -416,6 +416,7 @@ record_reg_classes (int n_alts, int n_op
 
   /* Process each alternative, each time minimizing an operand's cost
      with the cost for each operand in that alternative.  */
+  alternative_mask preferred = get_preferred_alternatives (insn);
   for (alt = 0; alt < n_alts; alt++)
     {
       enum reg_class classes[MAX_RECOG_OPERANDS];
@@ -424,7 +425,7 @@ record_reg_classes (int n_alts, int n_op
       int alt_fail = 0;
       int alt_cost = 0, op_cost_add;
 
-      if (!TEST_BIT (recog_data.enabled_alternatives, alt))
+      if (!TEST_BIT (preferred, alt))
 	{
 	  for (i = 0; i < recog_data.n_operands; i++)
 	    constraints[i] = skip_alternative (constraints[i]);
Index: gcc/ira.c
===================================================================
--- gcc/ira.c	2014-10-13 08:02:40.953138663 +0100
+++ gcc/ira.c	2014-10-17 15:47:34.353453512 +0100
@@ -1783,6 +1783,7 @@ ira_setup_alts (rtx_insn *insn, HARD_REG
   int commutative = -1;
 
   extract_insn (insn);
+  alternative_mask preferred = get_preferred_alternatives (insn);
   CLEAR_HARD_REG_SET (alts);
   insn_constraints.release ();
   insn_constraints.safe_grow_cleared (recog_data.n_operands
@@ -1812,7 +1813,7 @@ ira_setup_alts (rtx_insn *insn, HARD_REG
 	}
       for (nalt = 0; nalt < recog_data.n_alternatives; nalt++)
 	{
-	  if (!TEST_BIT (recog_data.enabled_alternatives, nalt)
+	  if (!TEST_BIT (preferred, nalt)
 	      || TEST_HARD_REG_BIT (alts, nalt))
 	    continue;
 
Index: gcc/postreload.c
===================================================================
--- gcc/postreload.c	2014-10-17 15:44:50.219398486 +0100
+++ gcc/postreload.c	2014-10-17 15:47:34.357453465 +0100
@@ -493,6 +493,7 @@ reload_cse_simplify_operands (rtx_insn *
 	  SET_HARD_REG_BIT (equiv_regs[i], REGNO (l->loc));
     }
 
+  alternative_mask preferred = get_preferred_alternatives (insn);
   for (i = 0; i < recog_data.n_operands; i++)
     {
       enum machine_mode mode;
@@ -566,7 +567,7 @@ reload_cse_simplify_operands (rtx_insn *
 		     alternative yet and the operand being replaced is not
 		     a cheap CONST_INT.  */
 		  if (op_alt_regno[i][j] == -1
-		      && TEST_BIT (recog_data.enabled_alternatives, j)
+		      && TEST_BIT (preferred, j)
 		      && reg_fits_class_p (testreg, rclass, 0, mode)
 		      && (!CONST_INT_P (recog_data.operand[i])
 			  || (set_src_cost (recog_data.operand[i],
Index: gcc/config/i386/i386.c
===================================================================
--- gcc/config/i386/i386.c	2014-10-13 08:02:40.985138241 +0100
+++ gcc/config/i386/i386.c	2014-10-17 15:47:34.349453560 +0100
@@ -5901,10 +5901,10 @@ ix86_legitimate_combined_insn (rtx_insn
 	  /* Operand has no constraints, anything is OK.  */
  	  win = !n_alternatives;
 
-	  alternative_mask enabled = recog_data.enabled_alternatives;
+	  alternative_mask preferred = get_preferred_alternatives (insn);
 	  for (j = 0; j < n_alternatives; j++, op_alt += n_operands)
 	    {
-	      if (!TEST_BIT (enabled, j))
+	      if (!TEST_BIT (preferred, j))
 		continue;
 	      if (op_alt[i].anything_ok
 		  || (op_alt[i].matches != -1
Index: gcc/ira-lives.c
===================================================================
--- gcc/ira-lives.c	2014-09-22 08:36:24.217790119 +0100
+++ gcc/ira-lives.c	2014-10-17 15:47:34.353453512 +0100
@@ -85,6 +85,10 @@ Software Foundation; either version 3, o
 /* The number of last call at which given allocno was saved.  */
 static int *allocno_saved_at_call;
 
+/* The value of get_preferred_alternatives for the current instruction,
+   supplemental to recog_data.  */
+static alternative_mask preferred_alternatives;
+
 /* Record the birth of hard register REGNO, updating hard_regs_live and
    hard reg conflict information for living allocnos.  */
 static void
@@ -641,10 +645,9 @@ check_and_make_def_conflict (int alt, in
       /* If there's any alternative that allows USE to match DEF, do not
 	 record a conflict.  If that causes us to create an invalid
 	 instruction due to the earlyclobber, reload must fix it up.  */
-      alternative_mask enabled = recog_data.enabled_alternatives;
       for (alt1 = 0; alt1 < recog_data.n_alternatives; alt1++)
 	{
-	  if (!TEST_BIT (enabled, alt1))
+	  if (!TEST_BIT (preferred_alternatives, alt1))
 	    continue;
 	  const operand_alternative *op_alt1
 	    = &recog_op_alt[alt1 * n_operands];
@@ -692,10 +695,9 @@ make_early_clobber_and_input_conflicts (
 
   int n_alternatives = recog_data.n_alternatives;
   int n_operands = recog_data.n_operands;
-  alternative_mask enabled = recog_data.enabled_alternatives;
   const operand_alternative *op_alt = recog_op_alt;
   for (alt = 0; alt < n_alternatives; alt++, op_alt += n_operands)
-    if (TEST_BIT (enabled, alt))
+    if (TEST_BIT (preferred_alternatives, alt))
       for (def = 0; def < n_operands; def++)
 	{
 	  def_cl = NO_REGS;
@@ -762,13 +764,13 @@ single_reg_class (const char *constraint
   enum constraint_num cn;
 
   cl = NO_REGS;
-  alternative_mask enabled = recog_data.enabled_alternatives;
+  alternative_mask preferred = preferred_alternatives;
   for (; (c = *constraints); constraints += CONSTRAINT_LEN (c, constraints))
     if (c == '#')
-      enabled &= ~ALTERNATIVE_BIT (0);
+      preferred &= ~ALTERNATIVE_BIT (0);
     else if (c == ',')
-      enabled >>= 1;
-    else if (enabled & 1)
+      preferred >>= 1;
+    else if (preferred & 1)
       switch (c)
 	{
 	case 'g':
@@ -851,13 +853,13 @@ ira_implicitly_set_insn_hard_regs (HARD_
 	  mode = (GET_CODE (op) == SCRATCH
 		  ? GET_MODE (op) : PSEUDO_REGNO_MODE (regno));
 	  cl = NO_REGS;
-	  alternative_mask enabled = recog_data.enabled_alternatives;
+	  alternative_mask preferred = preferred_alternatives;
 	  for (; (c = *p); p += CONSTRAINT_LEN (c, p))
 	    if (c == '#')
-	      enabled &= ~ALTERNATIVE_BIT (0);
+	      preferred &= ~ALTERNATIVE_BIT (0);
 	    else if (c == ',')
-	      enabled >>= 1;
-	    else if (enabled & 1)
+	      preferred >>= 1;
+	    else if (preferred & 1)
 	      {
 		cl = reg_class_for_constraint (lookup_constraint (p));
 		if (cl != NO_REGS)
@@ -1174,6 +1176,7 @@ process_bb_node_lives (ira_loop_tree_nod
 	      }
 
 	  extract_insn (insn);
+	  preferred_alternatives = get_preferred_alternatives (insn);
 	  preprocess_constraints (insn);
 	  process_single_reg_class_operands (false, freq);
 
Index: gcc/lra-int.h
===================================================================
--- gcc/lra-int.h	2014-09-22 08:36:23.153803537 +0100
+++ gcc/lra-int.h	2014-10-17 15:47:34.357453465 +0100
@@ -233,8 +233,8 @@ struct lra_insn_recog_data
      value can be NULL or points to array of the hard register numbers
      ending with a negative value.  */
   int *arg_hard_regs;
-  /* Alternative enabled for the insn.	NULL for debug insns.  */
-  alternative_mask enabled_alternatives;
+  /* Cached value of get_preferred_alternatives.  */
+  alternative_mask preferred_alternatives;
   /* The following member value is always NULL for a debug insn.  */
   struct lra_insn_reg *regs;
 };
Index: gcc/lra-constraints.c
===================================================================
--- gcc/lra-constraints.c	2014-10-03 11:05:47.521106593 +0100
+++ gcc/lra-constraints.c	2014-10-17 15:47:34.357453465 +0100
@@ -1680,14 +1680,14 @@ process_alt_operands (int only_alternati
      together, the second alternatives go together, etc.
 
      First loop over alternatives.  */
-  alternative_mask enabled = curr_id->enabled_alternatives;
+  alternative_mask preferred = curr_id->preferred_alternatives;
   if (only_alternative >= 0)
-    enabled &= ALTERNATIVE_BIT (only_alternative);
+    preferred &= ALTERNATIVE_BIT (only_alternative);
 
   for (nalt = 0; nalt < n_alternatives; nalt++)
     {
       /* Loop over operands for one constraint alternative.  */
-      if (!TEST_BIT (enabled, nalt))
+      if (!TEST_BIT (preferred, nalt))
 	continue;
 
       overall = losers = reject = reload_nregs = reload_sum = 0;
Index: gcc/lra.c
===================================================================
--- gcc/lra.c	2014-10-17 15:44:50.219398486 +0100
+++ gcc/lra.c	2014-10-17 15:47:34.357453465 +0100
@@ -917,7 +917,7 @@ lra_set_insn_recog_data (rtx_insn *insn)
       data->insn_static_data = &debug_insn_static_data;
       data->dup_loc = NULL;
       data->arg_hard_regs = NULL;
-      data->enabled_alternatives = ALL_ALTERNATIVES;
+      data->preferred_alternatives = ALL_ALTERNATIVES;
       data->operand_loc = XNEWVEC (rtx *, 1);
       data->operand_loc[0] = &INSN_VAR_LOCATION_LOC (insn);
       return data;
@@ -977,7 +977,7 @@ lra_set_insn_recog_data (rtx_insn *insn)
 	  = (insn_static_data->operand[i].constraint[0] == '=' ? OP_OUT
 	     : insn_static_data->operand[i].constraint[0] == '+' ? OP_INOUT
 	     : OP_IN);
-      data->enabled_alternatives = ALL_ALTERNATIVES;
+      data->preferred_alternatives = ALL_ALTERNATIVES;
       if (nop > 0)
 	{
 	  operand_alternative *op_alt = XCNEWVEC (operand_alternative,
@@ -1011,7 +1011,7 @@ lra_set_insn_recog_data (rtx_insn *insn)
 	  memcpy (locs, recog_data.dup_loc, n * sizeof (rtx *));
 	}
       data->dup_loc = locs;
-      data->enabled_alternatives = get_enabled_alternatives (insn);
+      data->preferred_alternatives = get_preferred_alternatives (insn);
       const operand_alternative *op_alt = preprocess_insn_constraints (icode);
       if (!insn_static_data->operand_alternative)
 	setup_operand_alternative (data, op_alt);
@@ -1202,27 +1202,7 @@ lra_update_insn_recog_data (rtx_insn *in
       n = insn_static_data->n_dups;
       if (n != 0)
 	memcpy (data->dup_loc, recog_data.dup_loc, n * sizeof (rtx *));
-#if HAVE_ATTR_enabled
-#ifdef ENABLE_CHECKING
-      {
-	int i;
-	alternative_mask enabled;
-
-	n = insn_static_data->n_alternatives;
-	enabled = data->enabled_alternatives;
-	lra_assert (n >= 0);
-	/* Cache the insn to prevent extract_insn call from
-	   get_attr_enabled.  */
-	recog_data.insn = insn;
-	for (i = 0; i < n; i++)
-	  {
-	    which_alternative = i;
-	    lra_assert (TEST_BIT (enabled, i)
-			== (bool) get_attr_enabled (insn));
-	  }
-      }
-#endif
-#endif
+      lra_assert (check_bool_attrs (insn));
     }
   return data;
 }

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

* [PATCH 3/5] Pass an alternative_mask to constrain_operands
  2014-10-17 14:47 [PATCH 0/5] Add preferred_for_{size,speed} attributes Richard Sandiford
  2014-10-17 14:48 ` [PATCH 1/5] Add recog_constrain_insn Richard Sandiford
  2014-10-17 14:51 ` [PATCH 2/5] Add preferred_for_{size,speed} attributes Richard Sandiford
@ 2014-10-17 14:52 ` Richard Sandiford
  2014-10-21 15:36   ` Vladimir Makarov
  2014-10-21 17:46   ` Jeff Law
  2014-10-17 14:55 ` [PATCH 4/5] Remove recog_data.enabled_alternatives Richard Sandiford
  2014-10-17 14:58 ` [PATCH 5/5] Use preferred_for_speed in i386.md Richard Sandiford
  4 siblings, 2 replies; 17+ messages in thread
From: Richard Sandiford @ 2014-10-17 14:52 UTC (permalink / raw)
  To: gcc-patches; +Cc: vmakarov, ubizjak, markus, Ganesh.Gopalasubramanian

After the previous patch there are cases where we want to constrain
operands to any enabled alternative and cases where we want to also take
size/speed preferences into account.  The former applies when
constraining an existing instruction (which might originally have been
in a block with a different size/speed choice) or when making global
decisions.  The latter applies when evaluating a potential optimisation.

This patch therefore passes the mask of allowable alternatives as a
parameter to constrain_operands.

Richard


gcc/
	* recog.h (constrain_operands): Add an alternative_mask parameter.
	(constrain_operands_cached): Likewise.
	(get_preferred_alternatives): Declare new form.
	* recog.c (get_preferred_alternatives): New bb-taking instance.
	(constrain_operands): Take the set of available alternatives as
	a parameter.
	(check_asm_operands, insn_invalid_p, extract_constrain_insn)
	(extract_constrain_insn_cached): Update calls to constrain_operands.
	* caller-save.c (reg_save_code): Likewise.
	* ira.c (setup_prohibited_mode_move_regs): Likewise.
	* postreload-gcse.c (eliminate_partially_redundant_load): Likewise.
	* ree.c (combine_reaching_defs): Likewise.
	* reload.c (can_reload_into): Likewise.
	* reload1.c (reload, reload_as_needed, inc_for_reload): Likewise.
	(gen_reload_chain_without_interm_reg_p, emit_input_reload_insns)
	(emit_insn_if_valid_for_reload): Likewise.
	* reorg.c (fill_slots_from_thread): Likewise.
	* config/i386/i386.c (ix86_attr_length_address_default): Likewise.
	* config/pa/pa.c (pa_can_combine_p): Likewise.
	* config/rl78/rl78.c (insn_ok_now): Likewise.
	* config/sh/sh.md (define_peephole2): Likewise.
	* final.c (final_scan_insn): Update call to constrain_operands_cached.

Index: gcc/recog.h
===================================================================
--- gcc/recog.h	2014-10-17 15:50:02.000000000 +0100
+++ gcc/recog.h	2014-10-17 15:50:02.627695847 +0100
@@ -95,8 +95,8 @@ extern void confirm_change_group (void);
 extern int apply_change_group (void);
 extern int num_validated_changes (void);
 extern void cancel_changes (int);
-extern int constrain_operands (int);
-extern int constrain_operands_cached (int);
+extern int constrain_operands (int, alternative_mask);
+extern int constrain_operands_cached (rtx_insn *, int);
 extern int memory_address_addr_space_p (enum machine_mode, rtx, addr_space_t);
 #define memory_address_p(mode,addr) \
 	memory_address_addr_space_p ((mode), (addr), ADDR_SPACE_GENERIC)
@@ -414,6 +414,7 @@ #define this_target_recog (&default_targ
 
 alternative_mask get_enabled_alternatives (rtx_insn *);
 alternative_mask get_preferred_alternatives (rtx_insn *);
+alternative_mask get_preferred_alternatives (rtx_insn *, basic_block);
 bool check_bool_attrs (rtx_insn *);
 
 void recog_init ();
Index: gcc/recog.c
===================================================================
--- gcc/recog.c	2014-10-17 15:50:02.000000000 +0100
+++ gcc/recog.c	2014-10-17 15:50:02.627695847 +0100
@@ -155,8 +155,9 @@ check_asm_operands (rtx x)
   if (reload_completed)
     {
       /* ??? Doh!  We've not got the wrapping insn.  Cook one up.  */
-      extract_insn (make_insn_raw (x));
-      constrain_operands (1);
+      rtx_insn *insn = make_insn_raw (x);
+      extract_insn (insn);
+      constrain_operands (1, get_enabled_alternatives (insn));
       return which_alternative >= 0;
     }
 
@@ -360,7 +361,7 @@ insn_invalid_p (rtx_insn *insn, bool in_
     {
       extract_insn (insn);
 
-      if (! constrain_operands (1))
+      if (! constrain_operands (1, get_preferred_alternatives (insn)))
 	return 1;
     }
 
@@ -2159,6 +2160,21 @@ get_preferred_alternatives (rtx_insn *in
     return get_bool_attr_mask (insn, BA_PREFERRED_FOR_SIZE);
 }
 
+/* Return the set of alternatives of INSN that are allowed by the current
+   target and are preferred for the size/speed optimization choice
+   associated with BB.  Passing a separate BB is useful if INSN has not
+   been emitted yet or if we are considering moving it to a different
+   block.  */
+
+alternative_mask
+get_preferred_alternatives (rtx_insn *insn, basic_block bb)
+{
+  if (optimize_bb_for_speed_p (bb))
+    return get_bool_attr_mask (insn, BA_PREFERRED_FOR_SPEED);
+  else
+    return get_bool_attr_mask (insn, BA_PREFERRED_FOR_SIZE);
+}
+
 /* Assert that the cached boolean attributes for INSN are still accurate.
    The backend is required to define these attributes in a way that only
    depends on the current target (rather than operands, compiler phase,
@@ -2199,7 +2215,7 @@ extract_insn_cached (rtx_insn *insn)
 extract_constrain_insn (rtx_insn *insn)
 {
   extract_insn (insn);
-  if (!constrain_operands (reload_completed))
+  if (!constrain_operands (reload_completed, get_enabled_alternatives (insn)))
     fatal_insn_not_found (insn);
 }
 
@@ -2210,16 +2226,17 @@ extract_constrain_insn_cached (rtx_insn
 {
   extract_insn_cached (insn);
   if (which_alternative == -1
-      && !constrain_operands (reload_completed))
+      && !constrain_operands (reload_completed,
+			      get_enabled_alternatives (insn)))
     fatal_insn_not_found (insn);
 }
 
-/* Do cached constrain_operands and complain about failures.  */
+/* Do cached constrain_operands on INSN and complain about failures.  */
 int
-constrain_operands_cached (int strict)
+constrain_operands_cached (rtx_insn *insn, int strict)
 {
   if (which_alternative == -1)
-    return constrain_operands (strict);
+    return constrain_operands (strict, get_enabled_alternatives (insn));
   else
     return 1;
 }
@@ -2495,7 +2512,8 @@ preprocess_constraints (rtx insn)
 }
 
 /* Check the operands of an insn against the insn's operand constraints
-   and return 1 if they are valid.
+   and return 1 if they match any of the alternatives in ALTERNATIVES.
+
    The information about the insn's operands, constraints, operand modes
    etc. is obtained from the global variables set up by extract_insn.
 
@@ -2527,7 +2545,7 @@ struct funny_match
 };
 
 int
-constrain_operands (int strict)
+constrain_operands (int strict, alternative_mask alternatives)
 {
   const char *constraints[MAX_RECOG_OPERANDS];
   int matching_operands[MAX_RECOG_OPERANDS];
@@ -2554,7 +2572,7 @@ constrain_operands (int strict)
       int lose = 0;
       funny_match_index = 0;
 
-      if (!TEST_BIT (recog_data.enabled_alternatives, which_alternative))
+      if (!TEST_BIT (alternatives, which_alternative))
 	{
 	  int i;
 
@@ -2836,7 +2854,7 @@ constrain_operands (int strict)
   /* If we are about to reject this, but we are not to test strictly,
      try a very loose test.  Only return failure if it fails also.  */
   if (strict == 0)
-    return constrain_operands (-1);
+    return constrain_operands (-1, alternatives);
   else
     return 0;
 }
Index: gcc/caller-save.c
===================================================================
--- gcc/caller-save.c	2014-10-17 15:50:02.000000000 +0100
+++ gcc/caller-save.c	2014-10-17 15:50:02.611696037 +0100
@@ -138,15 +138,17 @@ reg_save_code (int reg, enum machine_mod
   cached_reg_restore_code[reg][mode] = recog_memoized (restinsn);
 
   /* Now extract both insns and see if we can meet their
-     constraints.  */
+     constraints.  We don't know here whether the save and restore will
+     be in size- or speed-tuned code, so just use the set of enabled
+     alternatives.  */
   ok = (cached_reg_save_code[reg][mode] != -1
 	&& cached_reg_restore_code[reg][mode] != -1);
   if (ok)
     {
       extract_insn (saveinsn);
-      ok = constrain_operands (1);
+      ok = constrain_operands (1, get_enabled_alternatives (saveinsn));
       extract_insn (restinsn);
-      ok &= constrain_operands (1);
+      ok &= constrain_operands (1, get_enabled_alternatives (restinsn));
     }
 
   if (! ok)
Index: gcc/ira.c
===================================================================
--- gcc/ira.c	2014-10-17 15:50:02.000000000 +0100
+++ gcc/ira.c	2014-10-17 15:50:02.623695895 +0100
@@ -1760,7 +1760,9 @@ setup_prohibited_mode_move_regs (void)
 	  if (INSN_CODE (move_insn) < 0)
 	    continue;
 	  extract_insn (move_insn);
-	  if (! constrain_operands (1))
+	  /* We don't know whether the move will be in code that is optimized
+	     for size or speed, so consider all enabled alternatives.  */
+	  if (! constrain_operands (1, get_enabled_alternatives (move_insn)))
 	    continue;
 	  CLEAR_HARD_REG_BIT (ira_prohibited_mode_move_regs[i], j);
 	}
Index: gcc/postreload-gcse.c
===================================================================
--- gcc/postreload-gcse.c	2014-10-17 15:50:02.000000000 +0100
+++ gcc/postreload-gcse.c	2014-10-17 15:50:02.627695847 +0100
@@ -1003,10 +1003,11 @@ eliminate_partially_redundant_load (basi
 
 	  /* Make sure we can generate a move from register avail_reg to
 	     dest.  */
-	  extract_insn (as_a <rtx_insn *> (
-			  gen_move_insn (copy_rtx (dest),
-					 copy_rtx (avail_reg))));
-	  if (! constrain_operands (1)
+	  rtx_insn *move = as_a <rtx_insn *>
+	    (gen_move_insn (copy_rtx (dest), copy_rtx (avail_reg)));
+	  extract_insn (move);
+	  if (! constrain_operands (1, get_preferred_alternatives (insn,
+								   pred_bb))
 	      || reg_killed_on_edge (avail_reg, pred)
 	      || reg_used_on_edge (dest, pred))
 	    {
Index: gcc/ree.c
===================================================================
--- gcc/ree.c	2014-10-17 15:50:02.000000000 +0100
+++ gcc/ree.c	2014-10-17 15:50:02.627695847 +0100
@@ -762,7 +762,8 @@ combine_reaching_defs (ext_cand *cand, c
 	 This is merely to keep the test for safety and updating the insn
 	 stream simple.  Also ensure that within the block the candidate
 	 follows the defining insn.  */
-      if (BLOCK_FOR_INSN (cand->insn) != BLOCK_FOR_INSN (def_insn)
+      basic_block bb = BLOCK_FOR_INSN (cand->insn);
+      if (bb != BLOCK_FOR_INSN (def_insn)
 	  || DF_INSN_LUID (def_insn) > DF_INSN_LUID (cand->insn))
 	return false;
 
@@ -812,7 +813,7 @@ combine_reaching_defs (ext_cand *cand, c
       if (recog_memoized (insn) == -1)
 	return false;
       extract_insn (insn);
-      if (!constrain_operands (1))
+      if (!constrain_operands (1, get_preferred_alternatives (insn, bb)))
 	return false;
     }
 
Index: gcc/reload.c
===================================================================
--- gcc/reload.c	2014-10-17 15:50:02.000000000 +0100
+++ gcc/reload.c	2014-10-17 15:50:02.627695847 +0100
@@ -913,7 +913,7 @@ can_reload_into (rtx in, int regno, enum
   if (recog_memoized (test_insn) >= 0)
     {
       extract_insn (test_insn);
-      r = constrain_operands (1);
+      r = constrain_operands (1, get_enabled_alternatives (test_insn));
     }
   recog_data = save_recog_data;
   return r;
Index: gcc/reload1.c
===================================================================
--- gcc/reload1.c	2014-10-17 15:50:02.000000000 +0100
+++ gcc/reload1.c	2014-10-17 15:50:02.627695847 +0100
@@ -1257,7 +1257,7 @@ reload (rtx_insn *first, int global)
 	if (asm_noperands (PATTERN (insn)) >= 0)
 	  {
 	    extract_insn (insn);
-	    if (!constrain_operands (1))
+	    if (!constrain_operands (1, get_enabled_alternatives (insn)))
 	      {
 		error_for_asm (insn,
 			       "%<asm%> operand has impossible constraints");
@@ -4709,7 +4709,9 @@ reload_as_needed (int live_known)
 		  if (p != insn && INSN_P (p)
 		      && GET_CODE (PATTERN (p)) != USE
 		      && (recog_memoized (p) < 0
-			  || (extract_insn (p), ! constrain_operands (1))))
+			  || (extract_insn (p),
+			      !(constrain_operands (1,
+				  get_enabled_alternatives (p))))))
 		    {
 		      error_for_asm (insn,
 				     "%<asm%> operand requires "
@@ -4792,7 +4794,8 @@ reload_as_needed (int live_known)
 			      if (n)
 				{
 				  extract_insn (p);
-				  n = constrain_operands (1);
+				  n = constrain_operands (1,
+				    get_enabled_alternatives (p));
 				}
 
 			      /* If the constraints were not met, then
@@ -5719,7 +5722,7 @@ gen_reload_chain_without_interm_reg_p (i
 	  /* We want constrain operands to treat this insn strictly in
 	     its validity determination, i.e., the way it would after
 	     reload has completed.  */
-	  result = constrain_operands (1);
+	  result = constrain_operands (1, get_enabled_alternatives (insn));
 	}
 
       delete_insns_since (last);
@@ -7389,7 +7392,7 @@ emit_input_reload_insns (struct insn_cha
 	     autoincrement addressing mode, then the resulting insn
 	     is ill-formed and we must reject this optimization.  */
 	  extract_insn (temp);
-	  if (constrain_operands (1)
+	  if (constrain_operands (1, get_enabled_alternatives (temp))
 #ifdef AUTO_INC_DEC
 	      && ! find_reg_note (temp, REG_INC, reloadreg)
 #endif
@@ -8576,7 +8579,7 @@ emit_insn_if_valid_for_reload (rtx pat)
       /* We want constrain operands to treat this insn strictly in its
 	 validity determination, i.e., the way it would after reload has
 	 completed.  */
-      if (constrain_operands (1))
+      if (constrain_operands (1, get_enabled_alternatives (insn)))
 	return insn;
     }
 
@@ -9213,7 +9216,7 @@ inc_for_reload (rtx reloadreg, rtx in, r
       if (code >= 0)
 	{
 	  extract_insn (add_insn);
-	  if (constrain_operands (1))
+	  if (constrain_operands (1, get_enabled_alternatives (add_insn)))
 	    {
 	      /* If this is a pre-increment and we have incremented the value
 		 where it lives, copy the incremented value to RELOADREG to
Index: gcc/reorg.c
===================================================================
--- gcc/reorg.c	2014-10-17 15:50:02.000000000 +0100
+++ gcc/reorg.c	2014-10-17 15:50:02.627695847 +0100
@@ -2764,7 +2764,8 @@ fill_slots_from_thread (rtx_insn *insn,
 				   insn);
 
 	  if (recog_memoized (ninsn) < 0
-	      || (extract_insn (ninsn), ! constrain_operands (1)))
+	      || (extract_insn (ninsn),
+		  !constrain_operands (1, get_preferred_alternatives (ninsn))))
 	    {
 	      delete_related_insns (ninsn);
 	      return 0;
Index: gcc/config/i386/i386.c
===================================================================
--- gcc/config/i386/i386.c	2014-10-17 15:50:02.000000000 +0100
+++ gcc/config/i386/i386.c	2014-10-17 15:50:02.619695944 +0100
@@ -25300,7 +25300,7 @@ ix86_attr_length_address_default (rtx_in
   for (i = recog_data.n_operands - 1; i >= 0; --i)
     if (MEM_P (recog_data.operand[i]))
       {
-        constrain_operands_cached (reload_completed);
+        constrain_operands_cached (insn, reload_completed);
         if (which_alternative != -1)
 	  {
 	    const char *constraints = recog_data.constraints[i];
Index: gcc/config/pa/pa.c
===================================================================
--- gcc/config/pa/pa.c	2014-10-17 15:50:02.000000000 +0100
+++ gcc/config/pa/pa.c	2014-10-17 15:50:02.623695895 +0100
@@ -9199,8 +9199,10 @@ pa_can_combine_p (rtx_insn *new_rtx, rtx
   XVECEXP (PATTERN (new_rtx), 0, 1) = PATTERN (floater);
   INSN_CODE (new_rtx) = -1;
   insn_code_number = recog_memoized (new_rtx);
+  basic_block bb = BLOCK_FOR_INSN (anchor);
   if (insn_code_number < 0
-      || (extract_insn (new_rtx), ! constrain_operands (1)))
+      || (extract_insn (new_rtx),
+	  !constrain_operands (1, get_preferred_alternatives (new_rtx, bb)))
     return 0;
 
   if (reversed)
Index: gcc/config/rl78/rl78.c
===================================================================
--- gcc/config/rl78/rl78.c	2014-10-17 15:50:02.000000000 +0100
+++ gcc/config/rl78/rl78.c	2014-10-17 15:50:02.623695895 +0100
@@ -2165,7 +2165,7 @@ insn_ok_now (rtx_insn *insn)
   if (recog (pattern, insn, 0) > -1)
     {
       extract_insn (insn);
-      if (constrain_operands (1))
+      if (constrain_operands (1, get_preferred_alternatives (insn)))
 	{
 #if DEBUG_ALLOC
 	  fprintf (stderr, "\033[32m");
@@ -2194,7 +2194,7 @@ insn_ok_now (rtx_insn *insn)
       if (recog (pattern, insn, 0) > -1)
 	{
 	  extract_insn (insn);
-	  if (constrain_operands (0))
+	  if (constrain_operands (0, get_preferred_alternatives (insn)))
 	    {
 	      cfun->machine->virt_insns_ok = 0;
 	      return false;
Index: gcc/config/sh/sh.md
===================================================================
--- gcc/config/sh/sh.md	2014-10-17 15:50:02.000000000 +0100
+++ gcc/config/sh/sh.md	2014-10-17 15:50:02.623695895 +0100
@@ -1567,7 +1567,7 @@ (define_peephole2
    (set (match_dup 4) (match_dup 5))]
 {
   rtx set1, set2;
-  rtx_insn *insn2;
+  rtx_insn *insn1, *insn2;
   rtx replacements[4];
 
   /* We want to replace occurrences of operands[0] with operands[1] and
@@ -1594,14 +1594,16 @@ (define_peephole2
   /* ??? The last insn might be a jump insn, but the generic peephole2 code
      always uses emit_insn.  */
   /* Check that we don't violate matching constraints or earlyclobbers.  */
-  extract_insn (emit_insn (set1));
-  if (! constrain_operands (1))
+  basic_block bb = BLOCK_FOR_INSN (peep2_next_insn (2));
+  insn1 = emit_insn (set1);
+  extract_insn (insn1);
+  if (! constrain_operands (1, get_preferred_alternatives (insn1, bb)))
     goto failure;
   insn2 = emit (set2);
   if (GET_CODE (insn2) == BARRIER)
     goto failure;
   extract_insn (insn2);
-  if (! constrain_operands (1))
+  if (! constrain_operands (1, get_preferred_alternatives (insn2, bb)))
     {
       rtx tmp;
     failure:
Index: gcc/final.c
===================================================================
--- gcc/final.c	2014-10-17 15:50:02.000000000 +0100
+++ gcc/final.c	2014-10-17 15:50:02.623695895 +0100
@@ -2929,7 +2929,7 @@ final_scan_insn (rtx_insn *insn, FILE *f
 	    print_rtx_head = "";
 	  }
 
-	if (! constrain_operands_cached (1))
+	if (! constrain_operands_cached (insn, 1))
 	  fatal_insn_not_found (insn);
 
 	/* Some target machines need to prescan each insn before

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

* [PATCH 4/5] Remove recog_data.enabled_alternatives
  2014-10-17 14:47 [PATCH 0/5] Add preferred_for_{size,speed} attributes Richard Sandiford
                   ` (2 preceding siblings ...)
  2014-10-17 14:52 ` [PATCH 3/5] Pass an alternative_mask to constrain_operands Richard Sandiford
@ 2014-10-17 14:55 ` Richard Sandiford
  2014-10-21 14:57   ` Vladimir Makarov
  2014-10-21 15:35   ` Jeff Law
  2014-10-17 14:58 ` [PATCH 5/5] Use preferred_for_speed in i386.md Richard Sandiford
  4 siblings, 2 replies; 17+ messages in thread
From: Richard Sandiford @ 2014-10-17 14:55 UTC (permalink / raw)
  To: gcc-patches; +Cc: vmakarov, ubizjak, markus, Ganesh.Gopalasubramanian

After the previous patches, this one gets rid of
recog_data.enabled_alternatives and its one remaining use.

Richard


gcc/
	* recog.h (recog_data_d): Remove enabled_alternatives.
	* recog.c (extract_insn): Don't set it.
	* reload.c (find_reloads): Call get_enabled_alternatives.

Index: gcc/recog.h
===================================================================
--- gcc/recog.h	2014-10-17 15:50:02.627695847 +0100
+++ gcc/recog.h	2014-10-17 15:51:59.662308095 +0100
@@ -250,12 +250,6 @@ struct recog_data_d
   /* True if insn is ASM_OPERANDS.  */
   bool is_asm;
 
-  /* Specifies whether an insn alternative is enabled using the `enabled'
-     attribute in the insn pattern definition.  For back ends not using
-     the `enabled' attribute the bits are always set to 1 in expand_insn.
-     Bits beyond the last alternative are also set to 1.  */
-  alternative_mask enabled_alternatives;
-
   /* In case we are caching, hold insn data was generated for.  */
   rtx insn;
 };
Index: gcc/recog.c
===================================================================
--- gcc/recog.c	2014-10-17 15:50:02.627695847 +0100
+++ gcc/recog.c	2014-10-17 15:51:59.662308095 +0100
@@ -2339,8 +2339,6 @@ extract_insn (rtx_insn *insn)
 
   gcc_assert (recog_data.n_alternatives <= MAX_RECOG_ALTERNATIVES);
 
-  recog_data.enabled_alternatives = get_enabled_alternatives (insn);
-
   recog_data.insn = NULL;
   which_alternative = -1;
 }
Index: gcc/reload.c
===================================================================
--- gcc/reload.c	2014-10-17 15:50:02.627695847 +0100
+++ gcc/reload.c	2014-10-17 15:51:59.666308048 +0100
@@ -2997,13 +2997,14 @@ find_reloads (rtx_insn *insn, int replac
 
      First loop over alternatives.  */
 
+  alternative_mask enabled = get_enabled_alternatives (insn);
   for (this_alternative_number = 0;
        this_alternative_number < n_alternatives;
        this_alternative_number++)
     {
       int swapped;
 
-      if (!TEST_BIT (recog_data.enabled_alternatives, this_alternative_number))
+      if (!TEST_BIT (enabled, this_alternative_number))
 	{
 	  int i;
 

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

* [PATCH 5/5] Use preferred_for_speed in i386.md
  2014-10-17 14:47 [PATCH 0/5] Add preferred_for_{size,speed} attributes Richard Sandiford
                   ` (3 preceding siblings ...)
  2014-10-17 14:55 ` [PATCH 4/5] Remove recog_data.enabled_alternatives Richard Sandiford
@ 2014-10-17 14:58 ` Richard Sandiford
  2014-10-17 16:59   ` Uros Bizjak
  2014-10-21 15:08   ` Vladimir Makarov
  4 siblings, 2 replies; 17+ messages in thread
From: Richard Sandiford @ 2014-10-17 14:58 UTC (permalink / raw)
  To: gcc-patches; +Cc: vmakarov, ubizjak, markus, Ganesh.Gopalasubramanian

Undo the original fix for 61630 and use preferred_for_speed in the
problematic pattern.

I've not written many gcc.target/i386 tests so the markup might need
some work.

Richard


gcc/
	* lra.c (lra): Remove call to recog_init.
	* config/i386/i386.md (preferred_for_speed): New attribute
	(*float<SWI48:mode><MODEF:mode>2_sse): Override it instead of
	"enabled".

gcc/testsuite/
	* gcc.target/i386/conversion-2.c: New test.

Index: gcc/lra.c
===================================================================
--- gcc/lra.c	2014-10-17 15:47:34.357453465 +0100
+++ gcc/lra.c	2014-10-17 15:53:10.889463339 +0100
@@ -2116,11 +2116,6 @@ lra (FILE *f)
 
   lra_in_progress = 1;
 
-  /* The enable attributes can change their values as LRA starts
-     although it is a bad practice.  To prevent reuse of the outdated
-     values, clear them.  */
-  recog_init ();
-
   lra_live_range_iter = lra_coalesce_iter = 0;
   lra_constraint_iter = lra_constraint_iter_after_spill = 0;
   lra_inheritance_iter = lra_undo_inheritance_iter = 0;
Index: gcc/config/i386/i386.md
===================================================================
--- gcc/config/i386/i386.md	2014-10-01 10:48:51.079918153 +0100
+++ gcc/config/i386/i386.md	2014-10-17 15:53:10.889463339 +0100
@@ -779,6 +779,8 @@ (define_attr "enabled" ""
 	]
 	(const_int 1)))
 
+(define_attr "preferred_for_speed" "" (const_int 1))
+
 ;; Describe a user's asm statement.
 (define_asm_attributes
   [(set_attr "length" "128")
@@ -4794,16 +4796,12 @@ (define_insn "*float<SWI48:mode><MODEF:m
               (symbol_ref "TARGET_MIX_SSE_I387
                            && X87_ENABLE_FLOAT (<MODEF:MODE>mode,
                                                 <SWI48:MODE>mode)")
-            (eq_attr "alternative" "1")
-              /* ??? For sched1 we need constrain_operands to be able to
-                 select an alternative.  Leave this enabled before RA.  */
-              (symbol_ref "TARGET_INTER_UNIT_CONVERSIONS
-                           || optimize_function_for_size_p (cfun)
-                           || !(reload_completed
-                                || reload_in_progress
-                                || lra_in_progress)")
            ]
            (symbol_ref "true")))
+   (set (attr "preferred_for_speed")
+     (cond [(eq_attr "alternative" "1")
+              (symbol_ref "TARGET_INTER_UNIT_CONVERSIONS")]
+           (symbol_ref "true")))
    ])
 
 (define_insn "*float<SWI48x:mode><MODEF:mode>2_i387"
Index: gcc/testsuite/gcc.target/i386/conversion-2.c
===================================================================
--- /dev/null	2014-10-06 08:13:11.214126005 +0100
+++ gcc/testsuite/gcc.target/i386/conversion-2.c	2014-10-17 15:53:10.893463291 +0100
@@ -0,0 +1,35 @@
+/* { dg-options "-O2 -fno-toplevel-reorder -mfpmath=sse" } */
+/* { dg-require-effective-target lp64 } */
+
+void __attribute__ ((hot, target ("tune=bdver2")))
+f1 (int x)
+{
+  register float f asm ("%xmm0") = x;
+  asm volatile ("#f" :: "x" (f));
+}
+
+void __attribute__ ((cold, target ("tune=bdver2")))
+f2 (int x)
+{
+  register float f asm ("%xmm1") = x;
+  asm volatile ("#f" :: "x" (f));
+}
+
+void __attribute__ ((hot, target ("tune=bdver2")))
+f3 (int x)
+{
+  register float f asm ("%xmm2") = x;
+  asm volatile ("#f" :: "x" (f));
+}
+
+void __attribute__ ((cold, target ("tune=bdver2")))
+f4 (int x)
+{
+  register float f asm ("%xmm3") = x;
+  asm volatile ("#f" :: "x" (f));
+}
+
+/* { dg-final { scan-assembler "sp\\\), %xmm0" } } */
+/* { dg-final { scan-assembler "di, %xmm1" } } */
+/* { dg-final { scan-assembler "sp\\\), %xmm2" } } */
+/* { dg-final { scan-assembler "di, %xmm3" } } */

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

* Re: [PATCH 5/5] Use preferred_for_speed in i386.md
  2014-10-17 14:58 ` [PATCH 5/5] Use preferred_for_speed in i386.md Richard Sandiford
@ 2014-10-17 16:59   ` Uros Bizjak
  2014-10-21 15:08   ` Vladimir Makarov
  1 sibling, 0 replies; 17+ messages in thread
From: Uros Bizjak @ 2014-10-17 16:59 UTC (permalink / raw)
  To: gcc-patches, Vladimir Makarov, Uros Bizjak, Markus Trippelsdorf,
	Gopalasubramanian, Ganesh, richard.sandiford

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

On Fri, Oct 17, 2014 at 4:54 PM, Richard Sandiford
<richard.sandiford@arm.com> wrote:
> Undo the original fix for 61630 and use preferred_for_speed in the
> problematic pattern.
>
> I've not written many gcc.target/i386 tests so the markup might need
> some work.
>
> Richard
>
>
> gcc/
>         * lra.c (lra): Remove call to recog_init.
>         * config/i386/i386.md (preferred_for_speed): New attribute
>         (*float<SWI48:mode><MODEF:mode>2_sse): Override it instead of
>         "enabled".
>
> gcc/testsuite/
>         * gcc.target/i386/conversion-2.c: New test.

Please use the attached testcase that is also compatible with 32bit
compiles (and removes unnecessary comments in the asm). Please also
mention the revert  in ChangeLog.

OK for x86 part with these changes.

Thanks,
Uros.

[-- Attachment #2: conversion-2.c --]
[-- Type: text/x-csrc, Size: 841 bytes --]

/* { dg-do compile } */
/* { dg-options "-O2 -fno-toplevel-reorder -mtune=bdver2" } */
/* { dg-additional-options "-mregparm=1 -msse -mfpmath=sse" { target ia32 } } */

void __attribute__ ((hot))
f1 (int x)
{
  register float f asm ("%xmm0") = x;
  asm volatile ("" :: "x" (f));
}

void __attribute__ ((cold))
f2 (int x)
{
  register float f asm ("%xmm1") = x;
  asm volatile ("" :: "x" (f));
}

void __attribute__ ((hot))
f3 (int x)
{
  register float f asm ("%xmm2") = x;
  asm volatile ("" :: "x" (f));
}

void __attribute__ ((cold))
f4 (int x)
{
  register float f asm ("%xmm3") = x;
  asm volatile ("" :: "x" (f));
}

/* { dg-final { scan-assembler "sp\\\), %xmm0" } } */
/* { dg-final { scan-assembler "(ax|di), %xmm1" } } */
/* { dg-final { scan-assembler "sp\\\), %xmm2" } } */
/* { dg-final { scan-assembler "(ax|di), %xmm3" } } */

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

* Re: [PATCH 1/5] Add recog_constrain_insn
  2014-10-17 14:48 ` [PATCH 1/5] Add recog_constrain_insn Richard Sandiford
@ 2014-10-21 14:56   ` Vladimir Makarov
  2014-10-21 15:45   ` Jeff Law
  1 sibling, 0 replies; 17+ messages in thread
From: Vladimir Makarov @ 2014-10-21 14:56 UTC (permalink / raw)
  To: gcc-patches, ubizjak, markus, Ganesh.Gopalasubramanian,
	richard.sandiford

On 10/17/2014 10:47 AM, Richard Sandiford wrote:
> This patch just adds a new utility function called recog_constrain_insn,
> to go alongside the existing recog_constrain_insn_cached.
>
> Note that the extract_insn in lra.c wasn't used when checking is disabled.
> The function just moved on to the next instruction straight away.
>

The RA parts are ok for me.   Thanks, Richard.
>
> gcc/
> 	* recog.h (extract_constrain_insn): Declare.
> 	* recog.c (extract_constrain_insn): New function.
> 	* lra.c (check_rtl): Use it.
> 	* postreload.c (reload_cse_simplify_operands): Likewise.
> 	* reg-stack.c (check_asm_stack_operands): Likewise.
> 	(subst_asm_stack_regs): Likewise.
> 	* regcprop.c (copyprop_hardreg_forward_1): Likewise.
> 	* regrename.c (build_def_use): Likewise.
> 	* sel-sched.c (get_reg_class): Likewise.
> 	* config/arm/arm.c (note_invalid_constants): Likewise.
> 	* config/s390/predicates.md (execute_operation): Likewise.
>
>


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

* Re: [PATCH 2/5] Add preferred_for_{size,speed} attributes
  2014-10-17 14:51 ` [PATCH 2/5] Add preferred_for_{size,speed} attributes Richard Sandiford
@ 2014-10-21 14:57   ` Vladimir Makarov
  2014-10-21 17:34   ` Jeff Law
  2015-08-03 14:54   ` H.J. Lu
  2 siblings, 0 replies; 17+ messages in thread
From: Vladimir Makarov @ 2014-10-21 14:57 UTC (permalink / raw)
  To: gcc-patches, ubizjak, markus, Ganesh.Gopalasubramanian,
	richard.sandiford

On 10/17/2014 10:48 AM, Richard Sandiford wrote:
> This is the main patch, to add new preferred_for_size and
> preferred_for_speed attributes that can be used to selectively disable
> alternatives when optimising for size or speed.  As explained in the
> docs, the new attributes are just optimisation hints and it is possible
> that "size-only" alternatives will sometimes end up in a block that's
> optimised for speed, or vice versa.
>
> The patch deals with code that directly accesses the enabled_attributes
> mask and that ought to take size/speed choices into account.  The next
> patch deals with indirect uses.  Note that I'm not making reload support
> these attributes for hopefully obvious reasons :-)
>
> Richard
>
>
> gcc/
> 	* doc/md.texi: Document "preferred_for_size" and "preferred_for_speed"
> 	attributes.
> 	* genattr.c (main): Handle "preferred_for_size" and
> 	"preferred_for_speed" in the same way as "enabled".
> 	* recog.h (bool_attr): New enum.
> 	(target_recog): Replace x_enabled_alternatives with x_bool_attr_masks.
> 	(get_preferred_alternatives, check_bool_attrs): Declare.
> 	* recog.c (have_bool_attr, get_bool_attr, get_bool_attr_mask_uncached)
> 	(get_bool_attr_mask, get_preferred_alternatives, check_bool_attrs):
> 	New functions.
> 	(get_enabled_alternatives): Use get_bool_attr_mask.
> 	* ira-costs.c (record_reg_classes): Use get_preferred_alternatives
> 	instead of recog_data.enabled_alternatives.
> 	* ira.c (ira_setup_alts): Likewise.
> 	* postreload.c (reload_cse_simplify_operands): Likewise.
> 	* config/i386/i386.c (ix86_legitimate_combined_insn): Likewise.
> 	* ira-lives.c (preferred_alternatives): New variable.
> 	(process_bb_node_lives): Set it.
> 	(check_and_make_def_conflict, make_early_clobber_and_input_conflicts)
> 	(single_reg_class, ira_implicitly_set_insn_hard_regs): Use it instead
> 	of recog_data.enabled_alternatives.
> 	* lra-int.h (lra_insn_recog_data): Replace enabled_alternatives
> 	to preferred_alternatives.
> 	* lra-constraints.c (process_alt_operands): Update accordingly.
> 	* lra.c (lra_set_insn_recog_data): Likewise.
> 	(lra_update_insn_recog_data): Assert check_bool_attrs.
>
>
Thanks for picking this up and making a systematic solution, Richard. 
All  RA-related changes are ok for me.  I guess other changes
(genattrr.c, recog.[ch], md.texi and i386.c) are obvious but I have no
power to approve them.


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

* Re: [PATCH 4/5] Remove recog_data.enabled_alternatives
  2014-10-17 14:55 ` [PATCH 4/5] Remove recog_data.enabled_alternatives Richard Sandiford
@ 2014-10-21 14:57   ` Vladimir Makarov
  2014-10-21 15:35   ` Jeff Law
  1 sibling, 0 replies; 17+ messages in thread
From: Vladimir Makarov @ 2014-10-21 14:57 UTC (permalink / raw)
  To: gcc-patches, ubizjak, markus, Ganesh.Gopalasubramanian,
	richard.sandiford

On 10/17/2014 10:52 AM, Richard Sandiford wrote:
> After the previous patches, this one gets rid of
> recog_data.enabled_alternatives and its one remaining use.
>
>
Ok for me, too.  Pretty obvious patch although I have no power to
approve it all.

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

* Re: [PATCH 5/5] Use preferred_for_speed in i386.md
  2014-10-17 14:58 ` [PATCH 5/5] Use preferred_for_speed in i386.md Richard Sandiford
  2014-10-17 16:59   ` Uros Bizjak
@ 2014-10-21 15:08   ` Vladimir Makarov
  1 sibling, 0 replies; 17+ messages in thread
From: Vladimir Makarov @ 2014-10-21 15:08 UTC (permalink / raw)
  To: gcc-patches, ubizjak, markus, Ganesh.Gopalasubramanian,
	richard.sandiford

On 10/17/2014 10:54 AM, Richard Sandiford wrote:
> Undo the original fix for 61630 and use preferred_for_speed in the
> problematic pattern.
>
> I've not written many gcc.target/i386 tests so the markup might need
> some work.

Final lra.c change is ok for me too.
>
>
> gcc/
> 	* lra.c (lra): Remove call to recog_init.
> 	* config/i386/i386.md (preferred_for_speed): New attribute
> 	(*float<SWI48:mode><MODEF:mode>2_sse): Override it instead of
> 	"enabled".
>
> gcc/testsuite/
> 	* gcc.target/i386/conversion-2.c: New test.
>
>

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

* Re: [PATCH 4/5] Remove recog_data.enabled_alternatives
  2014-10-17 14:55 ` [PATCH 4/5] Remove recog_data.enabled_alternatives Richard Sandiford
  2014-10-21 14:57   ` Vladimir Makarov
@ 2014-10-21 15:35   ` Jeff Law
  1 sibling, 0 replies; 17+ messages in thread
From: Jeff Law @ 2014-10-21 15:35 UTC (permalink / raw)
  To: gcc-patches, vmakarov, ubizjak, markus, Ganesh.Gopalasubramanian,
	richard.sandiford

On 10/17/14 14:52, Richard Sandiford wrote:
> After the previous patches, this one gets rid of
> recog_data.enabled_alternatives and its one remaining use.
>
> Richard
>
>
> gcc/
> 	* recog.h (recog_data_d): Remove enabled_alternatives.
> 	* recog.c (extract_insn): Don't set it.
> 	* reload.c (find_reloads): Call get_enabled_alternatives.
This is fine once the prerequisites are approved.

jeff

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

* Re: [PATCH 3/5] Pass an alternative_mask to constrain_operands
  2014-10-17 14:52 ` [PATCH 3/5] Pass an alternative_mask to constrain_operands Richard Sandiford
@ 2014-10-21 15:36   ` Vladimir Makarov
  2014-10-21 17:46   ` Jeff Law
  1 sibling, 0 replies; 17+ messages in thread
From: Vladimir Makarov @ 2014-10-21 15:36 UTC (permalink / raw)
  To: gcc-patches, ubizjak, markus, Ganesh.Gopalasubramanian,
	richard.sandiford

On 10/17/2014 10:51 AM, Richard Sandiford wrote:
> After the previous patch there are cases where we want to constrain
> operands to any enabled alternative and cases where we want to also take
> size/speed preferences into account.  The former applies when
> constraining an existing instruction (which might originally have been
> in a block with a different size/speed choice) or when making global
> decisions.  The latter applies when evaluating a potential optimisation.
>
> This patch therefore passes the mask of allowable alternatives as a
> parameter to constrain_operands.
>
> Richard
>
RA parts look good for me too.
> gcc/
> 	* recog.h (constrain_operands): Add an alternative_mask parameter.
> 	(constrain_operands_cached): Likewise.
> 	(get_preferred_alternatives): Declare new form.
> 	* recog.c (get_preferred_alternatives): New bb-taking instance.
> 	(constrain_operands): Take the set of available alternatives as
> 	a parameter.
> 	(check_asm_operands, insn_invalid_p, extract_constrain_insn)
> 	(extract_constrain_insn_cached): Update calls to constrain_operands.
> 	* caller-save.c (reg_save_code): Likewise.
> 	* ira.c (setup_prohibited_mode_move_regs): Likewise.
> 	* postreload-gcse.c (eliminate_partially_redundant_load): Likewise.
> 	* ree.c (combine_reaching_defs): Likewise.
> 	* reload.c (can_reload_into): Likewise.
> 	* reload1.c (reload, reload_as_needed, inc_for_reload): Likewise.
> 	(gen_reload_chain_without_interm_reg_p, emit_input_reload_insns)
> 	(emit_insn_if_valid_for_reload): Likewise.
> 	* reorg.c (fill_slots_from_thread): Likewise.
> 	* config/i386/i386.c (ix86_attr_length_address_default): Likewise.
> 	* config/pa/pa.c (pa_can_combine_p): Likewise.
> 	* config/rl78/rl78.c (insn_ok_now): Likewise.
> 	* config/sh/sh.md (define_peephole2): Likewise.
> 	* final.c (final_scan_insn): Update call to constrain_operands_cached.
>


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

* Re: [PATCH 1/5] Add recog_constrain_insn
  2014-10-17 14:48 ` [PATCH 1/5] Add recog_constrain_insn Richard Sandiford
  2014-10-21 14:56   ` Vladimir Makarov
@ 2014-10-21 15:45   ` Jeff Law
  1 sibling, 0 replies; 17+ messages in thread
From: Jeff Law @ 2014-10-21 15:45 UTC (permalink / raw)
  To: gcc-patches, vmakarov, ubizjak, markus, Ganesh.Gopalasubramanian,
	richard.sandiford

On 10/17/14 14:47, Richard Sandiford wrote:
> This patch just adds a new utility function called recog_constrain_insn,
> to go alongside the existing recog_constrain_insn_cached.
>
> Note that the extract_insn in lra.c wasn't used when checking is disabled.
> The function just moved on to the next instruction straight away.
>
> Richard
>
>
> gcc/
> 	* recog.h (extract_constrain_insn): Declare.
> 	* recog.c (extract_constrain_insn): New function.
> 	* lra.c (check_rtl): Use it.
> 	* postreload.c (reload_cse_simplify_operands): Likewise.
> 	* reg-stack.c (check_asm_stack_operands): Likewise.
> 	(subst_asm_stack_regs): Likewise.
> 	* regcprop.c (copyprop_hardreg_forward_1): Likewise.
> 	* regrename.c (build_def_use): Likewise.
> 	* sel-sched.c (get_reg_class): Likewise.
> 	* config/arm/arm.c (note_invalid_constants): Likewise.
> 	* config/s390/predicates.md (execute_operation): Likewise.
OK.  I'm going to assume passing "reload_completed" is equivalent to the 
prior code where we passed the constant "1" to constrain_operands.

Jeff

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

* Re: [PATCH 2/5] Add preferred_for_{size,speed} attributes
  2014-10-17 14:51 ` [PATCH 2/5] Add preferred_for_{size,speed} attributes Richard Sandiford
  2014-10-21 14:57   ` Vladimir Makarov
@ 2014-10-21 17:34   ` Jeff Law
  2015-08-03 14:54   ` H.J. Lu
  2 siblings, 0 replies; 17+ messages in thread
From: Jeff Law @ 2014-10-21 17:34 UTC (permalink / raw)
  To: gcc-patches, vmakarov, ubizjak, markus, Ganesh.Gopalasubramanian,
	richard.sandiford

On 10/17/14 14:48, Richard Sandiford wrote:
> This is the main patch, to add new preferred_for_size and
> preferred_for_speed attributes that can be used to selectively disable
> alternatives when optimising for size or speed.  As explained in the
> docs, the new attributes are just optimisation hints and it is possible
> that "size-only" alternatives will sometimes end up in a block that's
> optimised for speed, or vice versa.
>
> The patch deals with code that directly accesses the enabled_attributes
> mask and that ought to take size/speed choices into account.  The next
> patch deals with indirect uses.  Note that I'm not making reload support
> these attributes for hopefully obvious reasons :-)
>
> Richard
>
>
> gcc/
> 	* doc/md.texi: Document "preferred_for_size" and "preferred_for_speed"
> 	attributes.
> 	* genattr.c (main): Handle "preferred_for_size" and
> 	"preferred_for_speed" in the same way as "enabled".
> 	* recog.h (bool_attr): New enum.
> 	(target_recog): Replace x_enabled_alternatives with x_bool_attr_masks.
> 	(get_preferred_alternatives, check_bool_attrs): Declare.
> 	* recog.c (have_bool_attr, get_bool_attr, get_bool_attr_mask_uncached)
> 	(get_bool_attr_mask, get_preferred_alternatives, check_bool_attrs):
> 	New functions.
> 	(get_enabled_alternatives): Use get_bool_attr_mask.
> 	* ira-costs.c (record_reg_classes): Use get_preferred_alternatives
> 	instead of recog_data.enabled_alternatives.
> 	* ira.c (ira_setup_alts): Likewise.
> 	* postreload.c (reload_cse_simplify_operands): Likewise.
> 	* config/i386/i386.c (ix86_legitimate_combined_insn): Likewise.
> 	* ira-lives.c (preferred_alternatives): New variable.
> 	(process_bb_node_lives): Set it.
> 	(check_and_make_def_conflict, make_early_clobber_and_input_conflicts)
> 	(single_reg_class, ira_implicitly_set_insn_hard_regs): Use it instead
> 	of recog_data.enabled_alternatives.
> 	* lra-int.h (lra_insn_recog_data): Replace enabled_alternatives
> 	to preferred_alternatives.
> 	* lra-constraints.c (process_alt_operands): Update accordingly.
> 	* lra.c (lra_set_insn_recog_data): Likewise.
> 	(lra_update_insn_recog_data): Assert check_bool_attrs.
>
OK for the trunk.

THanks,
Jeff

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

* Re: [PATCH 3/5] Pass an alternative_mask to constrain_operands
  2014-10-17 14:52 ` [PATCH 3/5] Pass an alternative_mask to constrain_operands Richard Sandiford
  2014-10-21 15:36   ` Vladimir Makarov
@ 2014-10-21 17:46   ` Jeff Law
  1 sibling, 0 replies; 17+ messages in thread
From: Jeff Law @ 2014-10-21 17:46 UTC (permalink / raw)
  To: gcc-patches, vmakarov, ubizjak, markus, Ganesh.Gopalasubramanian,
	richard.sandiford

On 10/17/14 14:51, Richard Sandiford wrote:
> After the previous patch there are cases where we want to constrain
> operands to any enabled alternative and cases where we want to also take
> size/speed preferences into account.  The former applies when
> constraining an existing instruction (which might originally have been
> in a block with a different size/speed choice) or when making global
> decisions.  The latter applies when evaluating a potential optimisation.
>
> This patch therefore passes the mask of allowable alternatives as a
> parameter to constrain_operands.
>
> Richard
>
>
> gcc/
> 	* recog.h (constrain_operands): Add an alternative_mask parameter.
> 	(constrain_operands_cached): Likewise.
> 	(get_preferred_alternatives): Declare new form.
> 	* recog.c (get_preferred_alternatives): New bb-taking instance.
> 	(constrain_operands): Take the set of available alternatives as
> 	a parameter.
> 	(check_asm_operands, insn_invalid_p, extract_constrain_insn)
> 	(extract_constrain_insn_cached): Update calls to constrain_operands.
> 	* caller-save.c (reg_save_code): Likewise.
> 	* ira.c (setup_prohibited_mode_move_regs): Likewise.
> 	* postreload-gcse.c (eliminate_partially_redundant_load): Likewise.
> 	* ree.c (combine_reaching_defs): Likewise.
> 	* reload.c (can_reload_into): Likewise.
> 	* reload1.c (reload, reload_as_needed, inc_for_reload): Likewise.
> 	(gen_reload_chain_without_interm_reg_p, emit_input_reload_insns)
> 	(emit_insn_if_valid_for_reload): Likewise.
> 	* reorg.c (fill_slots_from_thread): Likewise.
> 	* config/i386/i386.c (ix86_attr_length_address_default): Likewise.
> 	* config/pa/pa.c (pa_can_combine_p): Likewise.
> 	* config/rl78/rl78.c (insn_ok_now): Likewise.
> 	* config/sh/sh.md (define_peephole2): Likewise.
> 	* final.c (final_scan_insn): Update call to constrain_operands_cached.
>
OK.  Thanks for taking care of this.

Jeff


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

* Re: [PATCH 2/5] Add preferred_for_{size,speed} attributes
  2014-10-17 14:51 ` [PATCH 2/5] Add preferred_for_{size,speed} attributes Richard Sandiford
  2014-10-21 14:57   ` Vladimir Makarov
  2014-10-21 17:34   ` Jeff Law
@ 2015-08-03 14:54   ` H.J. Lu
  2 siblings, 0 replies; 17+ messages in thread
From: H.J. Lu @ 2015-08-03 14:54 UTC (permalink / raw)
  To: GCC Patches, Vladimir Makarov, Uros Bizjak, Markus Trippelsdorf,
	Gopalasubramanian, Ganesh, richard.sandiford

On Fri, Oct 17, 2014 at 7:48 AM, Richard Sandiford
<richard.sandiford@arm.com> wrote:
> This is the main patch, to add new preferred_for_size and
> preferred_for_speed attributes that can be used to selectively disable
> alternatives when optimising for size or speed.  As explained in the
> docs, the new attributes are just optimisation hints and it is possible
> that "size-only" alternatives will sometimes end up in a block that's
> optimised for speed, or vice versa.
>
> The patch deals with code that directly accesses the enabled_attributes
> mask and that ought to take size/speed choices into account.  The next
> patch deals with indirect uses.  Note that I'm not making reload support
> these attributes for hopefully obvious reasons :-)
>
> Richard
>
>
> gcc/
>         * doc/md.texi: Document "preferred_for_size" and "preferred_for_speed"
>         attributes.
>         * genattr.c (main): Handle "preferred_for_size" and
>         "preferred_for_speed" in the same way as "enabled".
>         * recog.h (bool_attr): New enum.
>         (target_recog): Replace x_enabled_alternatives with x_bool_attr_masks.
>         (get_preferred_alternatives, check_bool_attrs): Declare.
>         * recog.c (have_bool_attr, get_bool_attr, get_bool_attr_mask_uncached)
>         (get_bool_attr_mask, get_preferred_alternatives, check_bool_attrs):
>         New functions.
>         (get_enabled_alternatives): Use get_bool_attr_mask.
>         * ira-costs.c (record_reg_classes): Use get_preferred_alternatives
>         instead of recog_data.enabled_alternatives.
>         * ira.c (ira_setup_alts): Likewise.
>         * postreload.c (reload_cse_simplify_operands): Likewise.
>         * config/i386/i386.c (ix86_legitimate_combined_insn): Likewise.
>         * ira-lives.c (preferred_alternatives): New variable.
>         (process_bb_node_lives): Set it.
>         (check_and_make_def_conflict, make_early_clobber_and_input_conflicts)
>         (single_reg_class, ira_implicitly_set_insn_hard_regs): Use it instead
>         of recog_data.enabled_alternatives.
>         * lra-int.h (lra_insn_recog_data): Replace enabled_alternatives
>         to preferred_alternatives.
>         * lra-constraints.c (process_alt_operands): Update accordingly.
>         * lra.c (lra_set_insn_recog_data): Likewise.
>         (lra_update_insn_recog_data): Assert check_bool_attrs.
>

This caused:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67029

-- 
H.J.

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

end of thread, other threads:[~2015-08-03 14:54 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-17 14:47 [PATCH 0/5] Add preferred_for_{size,speed} attributes Richard Sandiford
2014-10-17 14:48 ` [PATCH 1/5] Add recog_constrain_insn Richard Sandiford
2014-10-21 14:56   ` Vladimir Makarov
2014-10-21 15:45   ` Jeff Law
2014-10-17 14:51 ` [PATCH 2/5] Add preferred_for_{size,speed} attributes Richard Sandiford
2014-10-21 14:57   ` Vladimir Makarov
2014-10-21 17:34   ` Jeff Law
2015-08-03 14:54   ` H.J. Lu
2014-10-17 14:52 ` [PATCH 3/5] Pass an alternative_mask to constrain_operands Richard Sandiford
2014-10-21 15:36   ` Vladimir Makarov
2014-10-21 17:46   ` Jeff Law
2014-10-17 14:55 ` [PATCH 4/5] Remove recog_data.enabled_alternatives Richard Sandiford
2014-10-21 14:57   ` Vladimir Makarov
2014-10-21 15:35   ` Jeff Law
2014-10-17 14:58 ` [PATCH 5/5] Use preferred_for_speed in i386.md Richard Sandiford
2014-10-17 16:59   ` Uros Bizjak
2014-10-21 15:08   ` Vladimir Makarov

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).