Re: [PATCH][RTL-ifcvt] Make non-conditional execution if-conversion more aggressive

[Jeff Law]

On 07/28/2015 04:14 AM, Kyrill Tkachov wrote:
[ Snip ]

Here's a respin.
I've reworked bbs_ok_for_cmove_arith to go over BB_A once and record
the set registers then go over BB_B once and look inside the SET_SRC
of each insn for those registers. How does this look? Would you like
me to investigate the data-flow infrastructure approach?

Also, in bb_valid_for_noce_process_p I iterate through the sub-rtxes
looking for a MEM with the FOR_EACH_SUBRTX machinery.

As I said above, I think moving the insns rather than re-emitting them
would make the function more convoluted than I think it needs to be.

Bootstrapped and tested on arm, aarch64, x86_64.



2015-07-28  Kyrylo Tkachov <kyrylo.tkac...@arm.com>

     * ifcvt.c (struct noce_if_info): Add then_simple, else_simple,
     then_cost, else_cost fields.  Change branch_cost field to unsigned
int.
     (end_ifcvt_sequence): Call set_used_flags on each insn in the
     sequence.
     (noce_simple_bbs): New function.
     (noce_try_move): Bail if basic blocks are not simple.
     (noce_try_store_flag): Likewise.
     (noce_try_store_flag_constants): Likewise.
     (noce_try_addcc): Likewise.
     (noce_try_store_flag_mask): Likewise.
     (noce_try_cmove): Likewise.
     (noce_try_minmax): Likewise.
     (noce_try_abs): Likewise.
     (noce_try_sign_mask): Likewise.
     (noce_try_bitop): Likewise.
     (bbs_ok_for_cmove_arith): New function.
     (noce_emit_all_but_last): Likewise.
     (noce_emit_insn): Likewise.
     (noce_emit_bb): Likewise.
     (noce_try_cmove_arith): Handle non-simple basic blocks.
     (insn_valid_noce_process_p): New function.
     (contains_mem_rtx_p): Likewise.
     (bb_valid_for_noce_process_p): Likewise.
     (noce_process_if_block): Allow non-simple basic blocks
     where appropriate.

2015-07-28  Kyrylo Tkachov <kyrylo.tkac...@arm.com>

     * gcc.dg/ifcvt-1.c: New test.
     * gcc.dg/ifcvt-2.c: Likewise.
     * gcc.dg/ifcvt-3.c: Likewise.
Thanks,
Kyrill

jeff



ifcvt.patch


commit a02417f015b70a0e47170ac7c41a5569392085a5
Author: Kyrylo Tkachov<kyrylo.tkac...@arm.com>
Date:   Wed Jul 8 15:45:04 2015 +0100

     [PATCH][ifcvt] Make non-conditional execution if-conversion more aggressive

diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index 2d97cc5..dae9b41 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -53,6 +53,7 @@
  #include "tree-pass.h"
  #include "dbgcnt.h"
  #include "shrink-wrap.h"
+#include "rtl-iter.h"
  #include "ifcvt.h"
Needs to be mentioned in the ChangeLog.

@@ -1585,6 +1632,116 @@ noce_try_cmove (struct noce_if_info *if_info)
    return FALSE;
  }

+/* Return true iff the registers that the insns in BB_A set do not
+   get used in BB_B.  */
+
+static bool
+bbs_ok_for_cmove_arith (basic_block bb_a, basic_block bb_b)
+{
+  rtx_insn *a_insn;
+  bitmap bba_sets = BITMAP_ALLOC (&reg_obstack);
+
+  FOR_BB_INSNS (bb_a, a_insn)
+    {
+      if (!active_insn_p (a_insn))
+       continue;
+
+      rtx sset_a = single_set (a_insn);
+
+      if (!sset_a)
+       {
+         BITMAP_FREE (bba_sets);
+         return false;
+       }
+
+      rtx dest_reg = SET_DEST (sset_a);
+      bitmap_set_bit (bba_sets, REGNO (dest_reg));
+    }
So there's a tight relationship between the implementation of 
bbs_ok_for_cmove_arith and insn_valid_noce_process_p.  If there wasn't, 
then we'd probably be looking to use note_stores and note_uses.

With that tight relationship, I think the implementations should be 
close together in the file and a comment in insn_valid_noce_process_p 
indicating that if insn_valid_noce_process_p is changed that the 
implementation of bbs_ok_for_cmove_arith may need to change as well.

One things I am a bit worried about is a SET_DEST that is a ZERO_EXTRACT 
or SUBREG.  Those are usually a read and a write.  So you'd need to 
filter them out earlier (noce_operand_ok?) or handle them in the second 
half of bbs_ok_for_cmove_arith.

Jeff