This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 5/5] Use ira_setup_alts for conflict detection


Vladimir Makarov <vmakarov@redhat.com> writes:
> On 2019-06-21 9:43 a.m., Richard Sandiford wrote:
>> make_early_clobber_and_input_conflicts records allocno conflicts
>> between inputs and earlyclobber outputs.  It (rightly) avoids
>> doing this for inputs that are explicitly allowed to match the
>> output due to matching constraints.
>>
>> The problem is that whether this matching is allowed varies
>> between alternatives.  At the moment the code avoids adding
>> a clobber if *any* enabled alternative allows the match,
>> even if some other operand makes that alternative impossible.
>>
>> The specific instance of this for SVE is that some alternatives
>> allow matched earlyclobbers when a third operand X is constant zero.
>> We should avoid adding conflicts when X really is constant zero,
>> but should ignore the match if X is nonzero or nonconstant.
>>
>> ira_setup_alts can already filter these alternatives out for us,
>> so all we need to do is use it in process_bb_node_lives.  The
>> preferred_alternatives variable is only used for this earlyclobber
>> detection, so no other check should be affected.
>>
>> With the previous patch to check the reject weight in ira_setup_alts,
>> this has the effect of ignoring expensive alternatives if we have
>> other valid alternatives with zero cost.  It seems reasonable to base
>> the heuristic on only the alternatives that we'd actually like to use,
>> but if this ends up being too aggressive, we could instead make the new
>> reject behaviour conditional and only use it for add_insn_allocno_copies.
>>
> This patch definitely improves the heuristics.
>
> The only missed part is a comment for preferred_alternatives
>
> /* The value of get_preferred_alternatives for the current instruction,
>     supplemental to recog_data.  */
> static alternative_mask preferred_alternatives;
>
> The comment becomes misleading after the patch.

Oops, thanks for noticing that.

> With changing the comment, the patch is ok for me.

Thanks, here's what I installed after testing on aarch64-linux-gnu
and x86_64-linux-gnu.

Richard


2019-07-01  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* ira-lives.c (process_bb_node_lives): Use ira_setup_alts.

Index: gcc/ira-lives.c
===================================================================
--- gcc/ira-lives.c	2019-06-29 17:20:49.000000000 +0100
+++ gcc/ira-lives.c	2019-07-01 09:33:14.026477923 +0100
@@ -80,8 +80,9 @@ 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.  */
+/* The value returned by ira_setup_alts for the current instruction;
+   i.e. the set of alternatives that we should consider to be likely
+   candidates during reloading.  */
 static alternative_mask preferred_alternatives;
 
 /* If non-NULL, the source operand of a register to register copy for which
@@ -1236,9 +1237,7 @@ process_bb_node_lives (ira_loop_tree_nod
 		  }
 	      }
 
-	  extract_insn (insn);
-	  preferred_alternatives = get_preferred_alternatives (insn);
-	  preprocess_constraints (insn);
+	  preferred_alternatives = ira_setup_alts (insn);
 	  process_single_reg_class_operands (false, freq);
 
 	  if (call_p)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]