Could a gatekeeper please review the attached change that enables more
if-conversion?

Here is the proposed log message:

Convert:
if (a && b)
  x = ...
To:
if (a)
  if (b)
    x = ...
To enable more if-conversion.

Without this transformation, WHIRL-lowering creates control flow
that WOPT-if-conversion can't handle.  With this transformation,
this now looks like any other if-conversion opportunity to WOPT.
So this transformation enables more if-conversion (i.e. replace
branch with conditional move).  No if-conversion code was touched -
only lowering code.




I have also attached a small test program that demonstrates when the
transformation occurs.  Thanks,

-David Coakley / AMD Open Source Compiler Engineering
Index: osprey/be/com/fb_whirl.h
===================================================================
--- osprey/be/com/fb_whirl.h	(revision 3639)
+++ osprey/be/com/fb_whirl.h	(working copy)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010 Advanced Micro Devices, Inc.  All Rights Reserved.
+ * Copyright (C) 2010-2011 Advanced Micro Devices, Inc.  All Rights Reserved.
  */
 
 /* -*- c++ -*-
@@ -271,6 +271,7 @@
 
   // Lower feedback info
 
+  void FB_split_cand_if ( WN *wn_outer_if, WN *wn_inner_if );
   void FB_lower_branch  ( WN *wn_br,   WN *wn_branch );
   void FB_lower_circuit ( WN *wn_cand, WN *wn_left_br, WN *wn_right_br );
   void FB_factor_circuit( WN *wn_left, WN *wn_right,
Index: osprey/be/com/wn_lower.cxx
===================================================================
--- osprey/be/com/wn_lower.cxx	(revision 3639)
+++ osprey/be/com/wn_lower.cxx	(working copy)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008-2010 Advanced Micro Devices, Inc.  All Rights Reserved.
+ * Copyright (C) 2008-2011 Advanced Micro Devices, Inc.  All Rights Reserved.
  */
 
 /*
@@ -118,6 +118,7 @@
 #include "targ_const_private.h" // for TCON_R4, TCON_R8, ..
 #endif
 #include "be_memop_annot.h"
+#include "config_wopt.h"        // for WOPT_Enable_Simple_If_Conv
 
 #ifdef TARG_X8664
 #include <ext/hash_set>
@@ -13613,6 +13614,112 @@
 }
 #endif
 
+/* Similar to above tree_has_cand_cior, but more specialized.  Check if:
+ * - tree's op is a CAND
+ * - tree has no other nested cand/cor/cselect
+ */
+static BOOL tree_has_only_one_cand (WN *tree)
+{
+  WN_ITER *wni;
+  WN *wn;
+  INT count = 0;
+
+  if (WN_operator(tree) != OPR_CAND)
+      return 0;
+  for (wni = WN_WALK_TreeIter (tree); 
+       wni != NULL;
+       wni = WN_WALK_TreeNext (wni))
+  {
+    wn = WN_ITER_wn (wni);
+    if (WN_operator_is(wn, OPR_CAND))
+	++count;
+    if (WN_operator_is(wn, OPR_CIOR)	||
+	WN_operator_is(wn, OPR_CSELECT))
+        return 0;
+  }
+  return (count == 1);
+}
+
+/* Very similar to CFG::wn_is_assign from opt_cfg.cxx.
+ * See if wn is a single assignment, since this is the condition under
+ * which WOPT will perform if_conversion.
+ * Ideally, this code would not have to be duplicated, but it seems
+ * difficult to re-structure WOPT to create a single helper function.
+ * If these functions get out of synch, it's only a performance issue - not
+ * a correctness issue.
+ */
+static BOOL wn_is_assign(WN *wn)
+{
+  WN *wn_first = WN_first(wn);
+  WN *wn_last  = WN_last(wn);
+  return (wn_first && (wn_first == wn_last)
+	  && OPERATOR_is_store(WN_operator(wn_first)));
+}
+
+/* ===================================================================
+ * In order to enable if-conversion in WOPT, tranform:
+ *
+ * From:
+ * if (a && b) {
+ *   x = ...
+ * }
+ * else {
+ *   <empty>
+ * }
+ *
+ * To:
+ * if (a) {
+ *   if (b) {
+ *     x = ...
+ *   }
+ *   else {
+ *      <empty>
+ *   }
+ * }
+ * else {
+ *   <empty>
+ * }
+ * ==================================================================
+ */
+static void lower_split_single_cand(WN * tree, LOWER_ACTIONS actions)
+{
+  FmtAssert(WN_operator(tree) == OPR_IF, ("Expect a if-condition"));
+
+  if (WN_block_nonempty(WN_then(tree)) &&
+      !WN_block_nonempty(WN_else(tree)) &&
+      tree_has_only_one_cand(WN_if_test(tree)) &&
+      wn_is_assign(WN_then(tree)))
+  {
+    WN *outer_then = WN_then(tree);
+    INT64 if_line = WN_Get_Linenum(tree);
+    INT64 then_line = WN_Get_Linenum(WN_then(tree));
+    INT64 else_line = WN_Get_Linenum(WN_else(tree));
+    WN *outer_cond = lower_copy_tree(WN_kid0(WN_if_test(tree)), actions);
+    WN *inner_cond = lower_copy_tree(WN_kid1(WN_if_test(tree)), actions);
+    WN_DELETE_Tree(WN_if_test(tree));
+    WN_if_test(tree) = outer_cond;
+    WN *inner_then_block = WN_CreateBlock();
+    WN_Set_Linenum(inner_then_block, if_line);
+    WN *inner_else_block = WN_CreateBlock();
+    WN_Set_Linenum(inner_else_block, else_line);
+    WN *inner_if = WN_CreateIf(inner_cond, inner_then_block, inner_else_block);
+    WN_Set_Linenum(inner_if, if_line);
+    WN * wn_next;
+    // Move statements from outer-then clause to new inner-then clause.
+    for (WN * wn_tmp = WN_first(outer_then); wn_tmp; wn_tmp = wn_next)
+    {
+      wn_next = WN_next(wn_tmp);
+      WN_INSERT_BlockLast(inner_then_block, lower_copy_tree(wn_tmp, actions));
+      WN_DELETE_FromBlock(outer_then, wn_tmp);
+    }
+    WN_INSERT_BlockLast(outer_then, inner_if);
+    if (Cur_PU_Feedback)
+    {
+      Cur_PU_Feedback->FB_split_cand_if( tree, inner_if );
+    }
+  }
+}
+
 /* ====================================================================
  *
  * WN *lower_if(WN *block, WN *tree, LOWER_ACTIONS actions, WN ** ret_next)
@@ -13658,7 +13765,18 @@
       lower_simp_if_flip(tree, actions, ret_next);
       lower_simp_bit_and(tree, actions);
   }
-  
+
+  // Check for special case, to enable more if-conversion.  Do not check if:
+  // - disabled (OPT_Lower_Splitsinglecand == 0)
+  // or
+  // - if-conversion not enabled
+  //      (Opt_Level < 2 || WOPT_Enable_Simple_If_Conv < 1)
+  if (Opt_Level >= 2 &&
+      OPT_Lower_Splitsinglecand && WOPT_Enable_Simple_If_Conv >= 1)
+  {
+    lower_split_single_cand(tree, actions);
+  }
+    
 #ifndef SHORTCIRCUIT_HACK
   if (Action(LOWER_IF))
 #else
Index: osprey/be/com/fb_whirl.cxx
===================================================================
--- osprey/be/com/fb_whirl.cxx	(revision 3639)
+++ osprey/be/com/fb_whirl.cxx	(working copy)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010 Advanced Micro Devices, Inc.  All Rights Reserved.
+ * Copyright (C) 2010-2011 Advanced Micro Devices, Inc.  All Rights Reserved.
  */
 
 /*
@@ -1417,6 +1417,81 @@
   }
 }
 
+// When transforming:
+// From:
+// if (a && b) {
+//   x = ...
+// }
+// else {
+//   <empty>
+// }
+// To:
+// if (a) {
+//   if (b) {
+//     x = ...
+//   }
+//   else {
+//      <empty>
+//   }
+// }
+// else {
+//   <empty>
+// }
+//
+// We adjust the frequency data as follows:
+// From:
+//                     if (a && b)
+//                  TK /         \ NT
+//                    /           \
+// To:
+// 
+//                     if (a)
+//               TKnew /     \ NTnew
+//                    /       \
+//                 if (b)
+//          TKinner /   \ NTinner
+//                 /     \
+// Where:
+// TOT = TK + NT
+// TK% = TK / TOT
+// TK%new = sqrt(TK%)
+// TKnew = TOT * TK%new
+// NTnew = TOT - TKnew
+// TKinner = TKnew * TK%new
+// NTinner = TKnew - TKinner
+// 
+void
+FEEDBACK::FB_split_cand_if( WN *wn_outer_if, WN *wn_inner_if )
+{
+  if ( _trace )
+    fprintf( TFile, "FEEDBACK::FB_split_cand(0x%p, 0x%p):\n",
+	     wn_outer_if, wn_inner_if );
+
+  Is_True( wn_outer_if != NULL && wn_inner_if != NULL,
+	   ( "FEEDBACK::FB_cplit_can expects non-NULL wn_outer_if and wn_inner_if" ) );
+
+  Is_True( WN_operator( wn_outer_if ) == OPR_IF &&
+	   WN_operator( wn_inner_if ) == OPR_IF,
+	   ( "FEEDBACK::FB_split_cand expects IF wn_outer_if and wn_inner_if" ) );
+
+  const FB_Info_Branch& info_branch = Query_branch( wn_outer_if );
+  FB_FREQ freq_tkn = info_branch.freq_taken;
+  FB_FREQ freq_not = info_branch.freq_not_taken;
+  FB_FREQ freq_total = freq_tkn + freq_not;
+  FB_FREQ freq_tkn_pct = freq_tkn / freq_total;
+  FB_FREQ freq_tkn_pct_new = freq_tkn_pct.sqrt();
+  FB_FREQ freq_tkn_new_outer = freq_total * freq_tkn_pct_new;
+  FB_FREQ freq_not_new_outer = freq_total - freq_tkn_new_outer;
+  Annot_branch( wn_outer_if, FB_Info_Branch( freq_tkn_new_outer,
+					     freq_not_new_outer,
+					     OPR_IF ) );
+  FB_FREQ freq_tkn_new_inner = freq_tkn_new_outer * freq_tkn_pct_new;
+  FB_FREQ freq_not_new_inner = freq_tkn_new_outer - freq_tkn_new_inner;
+  Annot_branch( wn_inner_if, FB_Info_Branch( freq_tkn_new_inner,
+					     freq_not_new_inner,
+					     OPR_IF ) );
+}
+
 // ====================================================================
 // The following procedures update the feedback information for whirl
 // nodes created or modified during lowering transformations.
Index: osprey/common/com/fb_freq.h
===================================================================
--- osprey/common/com/fb_freq.h	(revision 3639)
+++ osprey/common/com/fb_freq.h	(working copy)
@@ -97,6 +97,7 @@
 #ifndef fb_freq_INCLUDED
 #define fb_freq_INCLUDED
 
+#include <math.h>
 #include "defs.h"
 #ifndef ERRORS_INCLUDED
 #include "errors.h"
@@ -310,6 +311,17 @@
     return FB_FREQ_TYPE_BETTER( _type, freq._type );
   }
 
+  FB_FREQ sqrt() {
+    if ( Zero() && Exact() )
+      return FB_FREQ( FB_FREQ_TYPE_EXACT, 0.0 );
+    FB_FREQ_TYPE type = FB_FREQ_TYPE_COMBINE( _type, FB_FREQ_TYPE_GUESS );
+    if ( FB_FREQ_TYPE_NOT_KNOWN( type ) )
+      return FB_FREQ( type );
+    Is_True( _value >= 0, ( "FB_FREQ: negative value %ld", _value ) );
+    return FB_FREQ( type, ::sqrt((double)_value) );
+    return *this;
+  }
+  
   // Operators
 
   FB_FREQ operator+= ( const FB_FREQ freq ) {
Index: osprey/common/com/config_opt.cxx
===================================================================
--- osprey/common/com/config_opt.cxx	(revision 3639)
+++ osprey/common/com/config_opt.cxx	(working copy)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008-2010 Advanced Micro Devices, Inc.  All Rights Reserved.
+ * Copyright (C) 2008-2011 Advanced Micro Devices, Inc.  All Rights Reserved.
  */
 
 /*
@@ -228,6 +228,8 @@
 BOOL OPT_Reorg_Common_Set = FALSE;	/* ... option seen? */
 BOOL OPT_Unroll_Analysis = TRUE;	/* Enable unroll limitations? */
 BOOL OPT_Unroll_Analysis_Set = FALSE;	/* ... option seen? */
+BOOL OPT_Lower_Splitsinglecand = TRUE;
+BOOL OPT_Lower_Splitsinglecand_Set = FALSE;
 #if defined(TARG_NVISA)
 BOOL OPT_Lower_Speculate = TRUE;	/* speculate CAND/CIOR */
 #else
@@ -715,6 +717,10 @@
     0, 0, 0,    &OPT_Space,	NULL,
     "Bias optimizations to minimize code space" },
 
+  { OVK_BOOL,	OV_INTERNAL,	TRUE, "split_single_cand",		"",
+    0, 0, 0,	&OPT_Lower_Splitsinglecand, &OPT_Lower_Splitsinglecand_Set,
+    "Allow splitting of single CAND for enabling if_conversion" },
+
   { OVK_BOOL,	OV_INTERNAL,	TRUE, "speculate",		"",
     0, 0, 0,	&OPT_Lower_Speculate, &OPT_Lower_Speculate_Set,
     "Allow speculation for CAND/COR operators" },
Index: osprey/common/com/config.h
===================================================================
--- osprey/common/com/config.h	(revision 3639)
+++ osprey/common/com/config.h	(working copy)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008-2010 Advanced Micro Devices, Inc.  All Rights Reserved.
+ * Copyright (C) 2008-2011 Advanced Micro Devices, Inc.  All Rights Reserved.
  */
 
 /*
@@ -583,6 +583,7 @@
 extern BOOL OPT_unroll_times_overridden;
 extern INT32 OPT_unroll_size;
 extern BOOL OPT_unroll_size_overridden;
+extern BOOL OPT_Lower_Splitsinglecand;
 extern BOOL OPT_Lower_Speculate;
 extern BOOL OPT_Lower_Treeheight;
 extern BOOL OPT_Inline_Divide;
/* regular if-conversion should occur */
int
sub1 (int x)
{
  int z = 10;
  if (x != 13) {
    z -= 5;
  }
  return z;
}

/* if-conversion should occur, given new optimization */
int
sub2and (int x, int y)
{
  int z = 10;
  if (x != 28 && y != 29) {
    z -= 3;
  }
  return z;
}

/* regular if-conversion should occur - there are 2 moves in if condition */
int
sub2move (int x)
{
  int z = 10;
  int y = 20;
  if (x != 13) {
    z -= 5;
    y -=6;
  }
  return z+y;
}

/* optimization should not kick in - there are 2 moves in if condition */
int
sub2andmove (int x, int y)
{
  int z = 10;
  int z1 = 20;
  if (x != 28 && y != 29) {
    z -= 3;
    z1 -= 6;
  }
  return z+z1;
}

/* regular if-conversion should occur, the nested if is explicit in
   the source */
int
sub2nest (int x, int y)
{
  int z = 10;
  if (x != 28) {
    if (y != 29) {
      z -= 3;
    }
  }
  return z;
}

/* optimization should not kick in, because of the ! outside the && */
int
sub2andnot (int x, int y)
{
  int z = 10;
  if (!(x == 28 && y == 29)) {
    z -= 3;
  }
  return z;
}

/* optimization should not kick in, because of nested condition */
int
sub2and3nest (int x, int y, int a)
{
  int z = 10;
  if (x == 28 && (y == 29 || a == 30)) {
    z -= 3;
  }
  return z;
}

/* optimization should not kick in, because of 2 && */
int
sub3and (int x, int y, int a)
{
  int z = 10;
  if (x == 28 && y == 29 && a == 30) {
    z -= 3;
  }
  return z;
}

/* optimization should not kick in, else block not empty */
int
sub2andelse (int x, int y)
{
  int z = 10;
  if (x != 28 && y != 29) {
    z -= 3;
  }
  else {
    z -= 6;
  }
  return z;
}
#include <stdio.h>

extern int sub1(int x);
extern int sub2and(int x, int y);
extern int sub2move(int x);
extern int sub2andmove(int x, int y);
extern int sub2nest(int x, int y);
extern int sub2andnot(int x, int y);
extern int sub2and3nest(int x, int y, int a);
extern int sub3and(int x, int y, int a);
extern int sub2andelse(int x, int y);

main ()
{
  if (sub1(10) != 5)
    printf("Error in sub1\n");
  if (sub2and(10, 11) != 7)
    printf("Error in sub2and\n");
  if (sub2move(10) != 19)
    printf("Error in sub2move\n");
  if (sub2andmove(10, 21) != 21)
    printf("Error in sub2andmove\n");
  if (sub2nest(10, 11) != 7)
    printf("Error in sub2nest\n");
  if (sub2andnot(10, 11) != 7)
    printf("Error in sub2andnot\n");
  if (sub2and3nest(28, 29, 1) != 7)
    printf("Error in sub2and3nest\n");
  if (sub3and(28, 29, 30) != 7)
    printf("Error in sub3and\n");
  if (sub2andelse(10, 11) != 7)
    printf("Error in sub2andelse\n");
}
------------------------------------------------------------------------------
Simplify data backup and recovery for your virtual environment with vRanger.
Installation's a snap, and flexible recovery options mean your data is safe,
secure and there when you need it. Discover what all the cheering's about.
Get your free trial download today. 
http://p.sf.net/sfu/quest-dev2dev2 
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel

Reply via email to