Re: [PATCH, PR43920, 6/9] Cross-jumping - Use reg-notes.

2011-04-06 Thread Jeff Law
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 04/01/11 08:54, Tom de Vries wrote:
 On 03/31/2011 11:16 PM, Tom de Vries wrote:
 On 03/31/2011 08:52 PM, Jeff Law wrote:

 On 03/31/11 12:42, Tom de Vries wrote:
 Uses regnotes to analyze whether we can replace insn a by insn b, even
 if we cannot replace insn b by insn a. Uses this info in crossjumping.

 Shouldn't this be using single_set rather than digging through PATTERN,
 then verifying both are SETs, etc.?

 Otherwise don't you miss most of the benefit on architectures where most
 insns clobber the flags register in a PARALLEL with the SET?

 I see what you mean about missing these insns currently.

 I guess I will have to check that the non-SET part of the PARALLEL is
 identical between the 2 insns.

 I'll update the patch to handle this case.
 
 changes compared to previous posting:
 - add ChangeLog.
 - use single_set
 - add equal_different_set_p and use it in can_replace_by
 
 Retested on x86_64.


   PR target/43920
   * cfgcleanup.c (equal_different_set_p, can_replace_by, merge_dir): New
   function.
   (old_insns_match_p): Change return type.  Replace return false/true with
   return dir_none/dir_both.  Use can_replace_by.
   (flow_find_cross_jump): Add dir_p parameter.  Init replacement direction
   from dir_p.  Register replacement direction in dir, last_dir and
   afterlast_dir.  Handle new return type of old_insns_match_p using
   merge_dir.  Return replacement direction in dir_p.
   (flow_find_head_matching_sequence, outgoing_edges_match): Handle new
   return type of old_insns_match_p.
   (try_crossjump_to_edge): Add argument to call to flow_find_cross_jump.
   * ifcvt.c ( cond_exec_process_if_block): Add argument to call to
   flow_find_cross_jump.
   * basic-block.h (enum replace_direction): New type.
   (flow_find_cross_jump): Add parameter to declaration.
OK

Jeff
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJNnKW/AAoJEBRtltQi2kC7TC8IAJdB1hgkPmmC787EUBBycPCF
/ROYeWMZ62WyVqOD+eTVFXvv6v4s0XjPHQgS+zANBQPdvA3L8V2ugFYy66SWmQZj
1NSplCrBMRhS9Fu9M8uEWjvuVEUhqxOYLnPKXqeW/gD8UEHt2+gMLAGGFI4pxQRS
L+caqVMGvNvVZqMNAUTU7FLQfT1Zo50sBvbvm9w/GfjSVNC/dmkHRqf4Ta0oIDW/
Zm5oyX4FWzun8NbW+scaQlsxAiEA5xoxzXyGlLnj9UGCTiEeaYIsgg+SyYO8CeO0
o3FpsRfe+jMSK170cd7+mufPktjmCuAdiWQa2M7W6R04AOvdOV7DxNglHMHXljg=
=NrNE
-END PGP SIGNATURE-


Re: [PATCH, PR43920, 6/9] Cross-jumping - Use reg-notes.

2011-04-04 Thread Tom de Vries
Hi Jeff,

On 04/01/2011 04:54 PM, Tom de Vries wrote:
 On 03/31/2011 11:16 PM, Tom de Vries wrote:
 On 03/31/2011 08:52 PM, Jeff Law wrote:

 On 03/31/11 12:42, Tom de Vries wrote:
 Uses regnotes to analyze whether we can replace insn a by insn b, even
 if we cannot replace insn b by insn a. Uses this info in crossjumping.

 Shouldn't this be using single_set rather than digging through PATTERN,
 then verifying both are SETs, etc.?

 Otherwise don't you miss most of the benefit on architectures where most
 insns clobber the flags register in a PARALLEL with the SET?

 I see what you mean about missing these insns currently.

 I guess I will have to check that the non-SET part of the PARALLEL is
 identical between the 2 insns.

 I'll update the patch to handle this case.
 
 changes compared to previous posting:
 - add ChangeLog.
 - use single_set
 - add equal_different_set_p and use it in can_replace_by
 
 Retested on x86_64.
 

Do these changes (
http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00038.html ) address your
concerns? Is the patch OK for trunk now?

Thanks,
- Tom


Re: [PATCH, PR43920, 6/9] Cross-jumping - Use reg-notes.

2011-04-01 Thread Tom de Vries
On 03/31/2011 11:16 PM, Tom de Vries wrote:
 On 03/31/2011 08:52 PM, Jeff Law wrote:
 
 On 03/31/11 12:42, Tom de Vries wrote:
 Uses regnotes to analyze whether we can replace insn a by insn b, even
 if we cannot replace insn b by insn a. Uses this info in crossjumping.
 
 Shouldn't this be using single_set rather than digging through PATTERN,
 then verifying both are SETs, etc.?

 Otherwise don't you miss most of the benefit on architectures where most
 insns clobber the flags register in a PARALLEL with the SET?
 
 I see what you mean about missing these insns currently.
 
 I guess I will have to check that the non-SET part of the PARALLEL is
 identical between the 2 insns.
 
 I'll update the patch to handle this case.

changes compared to previous posting:
- add ChangeLog.
- use single_set
- add equal_different_set_p and use it in can_replace_by

Retested on x86_64.

Thanks,
- Tom
2011-04-01  Tom de Vries  t...@codesourcery.com

	PR target/43920
	* cfgcleanup.c (equal_different_set_p, can_replace_by, merge_dir): New
	function.
	(old_insns_match_p): Change return type.  Replace return false/true with
	return dir_none/dir_both.  Use can_replace_by.
	(flow_find_cross_jump): Add dir_p parameter.  Init replacement direction
	from dir_p.  Register replacement direction in dir, last_dir and
	afterlast_dir.	Handle new return type of old_insns_match_p using
	merge_dir.  Return replacement direction in dir_p.
	(flow_find_head_matching_sequence, outgoing_edges_match): Handle new
	return type of old_insns_match_p.
	(try_crossjump_to_edge): Add argument to call to flow_find_cross_jump.
	* ifcvt.c ( cond_exec_process_if_block): Add argument to call to
	flow_find_cross_jump.
	* basic-block.h (enum replace_direction): New type.
	(flow_find_cross_jump): Add parameter to declaration.

diff -u gcc/cfgcleanup.c gcc/cfgcleanup.c
--- gcc/cfgcleanup.c	(working copy)
+++ gcc/cfgcleanup.c	(working copy)
@@ -72,7 +72,7 @@
 static bool try_crossjump_to_edge (int, edge, edge);
 static bool try_crossjump_bb (int, basic_block);
 static bool outgoing_edges_match (int, basic_block, basic_block);
-static bool old_insns_match_p (int, rtx, rtx);
+static enum replace_direction old_insns_match_p (int, rtx, rtx);
 
 static void merge_blocks_move_predecessor_nojumps (basic_block, basic_block);
 static void merge_blocks_move_successor_nojumps (basic_block, basic_block);
@@ -950,27 +950,143 @@
 }
 
 
+ /* Checks if patterns P1 and P2 are equivalent, apart from the possibly
+different single sets S1 and S2.  */
+
+static bool
+equal_different_set_p (rtx p1, rtx s1, rtx p2, rtx s2)
+{
+  int i;
+  rtx e1, e2;
+
+  if (p1 == s1  p2 == s2)
+return true;
+
+  if (GET_CODE (p1) != PARALLEL || GET_CODE (p2) != PARALLEL)
+return false;
+
+  if (XVECLEN (p1, 0) != XVECLEN (p2, 0))
+return false;
+
+  for (i = 0; i  XVECLEN (p1, 0); i++)
+{
+  e1 = XVECEXP (p1, 0, i);
+  e2 = XVECEXP (p2, 0, i);
+  if (e1 == s1  e2 == s2)
+continue;
+  if (reload_completed
+  ? rtx_renumbered_equal_p (e1, e2) : rtx_equal_p (e1, e2))
+continue;
+
+return false;
+}
+
+  return true;
+}
+
+/* Examine register notes on I1 and I2 and return:
+   - dir_forward if I1 can be replaced by I2, or
+   - dir_backward if I2 can be replaced by I1, or
+   - dir_both if both are the case.  */
+
+static enum replace_direction
+can_replace_by (rtx i1, rtx i2)
+{
+  rtx s1, s2, d1, d2, src1, src2, note1, note2;
+  bool c1, c2;
+
+  /* Check for 2 sets.  */
+  s1 = single_set (i1);
+  s2 = single_set (i2);
+  if (s1 == NULL_RTX || s2 == NULL_RTX)
+return dir_none;
+
+  /* Check that the 2 sets set the same dest.  */
+  d1 = SET_DEST (s1);
+  d2 = SET_DEST (s2);
+  if (!(reload_completed
+? rtx_renumbered_equal_p (d1, d2) : rtx_equal_p (d1, d2)))
+return dir_none;
+
+  /* Find identical req_equiv or reg_equal note, which implies that the 2 sets
+ set dest to the same value.  */
+  note1 = find_reg_equal_equiv_note (i1);
+  note2 = find_reg_equal_equiv_note (i2);
+  if (!note1 || !note2 || !rtx_equal_p (XEXP (note1, 0), XEXP (note2, 0))
+  || !CONST_INT_P (XEXP (note1, 0)))
+return dir_none;
+
+  if (!equal_different_set_p (PATTERN (i1), s1, PATTERN (i2), s2))
+return dir_none;
+
+  /* Although the 2 sets set dest to the same value, we cannot replace
+   (set (dest) (const_int))
+ by
+   (set (dest) (reg))
+ because we don't know if the reg is live and has the same value at the
+ location of replacement.  */
+  src1 = SET_SRC (s1);
+  src2 = SET_SRC (s2);
+  c1 = CONST_INT_P (src1);
+  c2 = CONST_INT_P (src2);
+  if (c1  c2)
+return dir_both;
+  else if (c2)
+return dir_forward;
+  else if (c1)
+return dir_backward;
+
+  return dir_none;
+}
+
+/* Merges directions A and B.  */
+
+static enum replace_direction
+merge_dir (enum replace_direction a, enum replace_direction b)
+{
+  /* Implements the following table:
+|bo fw bw no
+ ---+---
+