Re: [PATCH] bitint, v2: Fix up adjustment of large/huge _BitInt arguments of returns_twice calls [PR113466]

[Richard Biener <rguenther@suse.de>]

On Thu, 14 Mar 2024, Jakub Jelinek wrote:

> On Thu, Mar 14, 2024 at 09:59:12AM +0100, Richard Biener wrote:
> > On Thu, 14 Mar 2024, Jakub Jelinek wrote:
> > 
> > > On Thu, Mar 14, 2024 at 09:48:45AM +0100, Richard Biener wrote:
> > > > Ugh.  OK, but I wonder whether we might want to simply delay
> > > > fixing the CFG for inserts before returns-twice?  Would that make
> > > > things less ugly?
> > > 
> > > You mean in lower_call just remember if we added anything before
> > > ECF_RETURNS_TWICE call (or say add such stmt into a vector) and
> > > then fix it up before the end of the pass?
> > 
> > Yeah, or just walk BBs with abnormal preds, whatever tracking is
> > easier.
> 
> Walking all the bbs with abnormal preds would mean I'd have to walk their
> whole bodies, because the ECF_RETURNS_TWICE call is no longer at the start.
> 
> The following patch seems to work, ok for trunk if it passes full
> bootstrap/regtest?

OK.  I'll let you decide which variant is better maintainable
(I think this one).

Thanks,
Richard.

> 2024-03-14  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/113466
> 	* gimple-lower-bitint.cc (bitint_large_huge): Add m_returns_twice_calls
> 	member.
> 	(bitint_large_huge::bitint_large_huge): Initialize it.
> 	(bitint_large_huge::~bitint_large_huge): Release it.
> 	(bitint_large_huge::lower_call): Remember ECF_RETURNS_TWICE call stmts
> 	before which at least one statement has been inserted.
> 	(gimple_lower_bitint): Move argument loads before ECF_RETURNS_TWICE
> 	calls to a different block and add corresponding PHIs.
> 
> 	* gcc.dg/bitint-100.c: New test.
> 
> --- gcc/gimple-lower-bitint.cc.jj	2024-03-13 15:34:29.969725763 +0100
> +++ gcc/gimple-lower-bitint.cc	2024-03-14 11:25:07.763169074 +0100
> @@ -419,7 +419,8 @@ struct bitint_large_huge
>    bitint_large_huge ()
>      : m_names (NULL), m_loads (NULL), m_preserved (NULL),
>        m_single_use_names (NULL), m_map (NULL), m_vars (NULL),
> -      m_limb_type (NULL_TREE), m_data (vNULL) {}
> +      m_limb_type (NULL_TREE), m_data (vNULL),
> +      m_returns_twice_calls (vNULL) {}
>  
>    ~bitint_large_huge ();
>  
> @@ -553,6 +554,7 @@ struct bitint_large_huge
>    unsigned m_bitfld_load;
>    vec<tree> m_data;
>    unsigned int m_data_cnt;
> +  vec<gimple *> m_returns_twice_calls;
>  };
>  
>  bitint_large_huge::~bitint_large_huge ()
> @@ -565,6 +567,7 @@ bitint_large_huge::~bitint_large_huge ()
>      delete_var_map (m_map);
>    XDELETEVEC (m_vars);
>    m_data.release ();
> +  m_returns_twice_calls.release ();
>  }
>  
>  /* Insert gimple statement G before current location
> @@ -5248,6 +5251,7 @@ bitint_large_huge::lower_call (tree obj,
>        default:
>  	break;
>        }
> +  bool returns_twice = (gimple_call_flags (stmt) & ECF_RETURNS_TWICE) != 0;
>    for (unsigned int i = 0; i < nargs; ++i)
>      {
>        tree arg = gimple_call_arg (stmt, i);
> @@ -5271,6 +5275,11 @@ bitint_large_huge::lower_call (tree obj,
>  	  arg = make_ssa_name (TREE_TYPE (arg));
>  	  gimple *g = gimple_build_assign (arg, v);
>  	  gsi_insert_before (&gsi, g, GSI_SAME_STMT);
> +	  if (returns_twice)
> +	    {
> +	      m_returns_twice_calls.safe_push (stmt);
> +	      returns_twice = false;
> +	    }
>  	}
>        gimple_call_set_arg (stmt, i, arg);
>        if (m_preserved == NULL)
> @@ -7091,6 +7100,66 @@ gimple_lower_bitint (void)
>    if (edge_insertions)
>      gsi_commit_edge_inserts ();
>  
> +  /* Fix up arguments of ECF_RETURNS_TWICE calls.  Those were temporarily
> +     inserted before the call, but that is invalid IL, so move them to the
> +     right place and add corresponding PHIs.  */
> +  if (!large_huge.m_returns_twice_calls.is_empty ())
> +    {
> +      auto_vec<gimple *, 16> arg_stmts;
> +      while (!large_huge.m_returns_twice_calls.is_empty ())
> +	{
> +	  gimple *stmt = large_huge.m_returns_twice_calls.pop ();
> +	  gimple_stmt_iterator gsi = gsi_after_labels (gimple_bb (stmt));
> +	  while (gsi_stmt (gsi) != stmt)
> +	    {
> +	      arg_stmts.safe_push (gsi_stmt (gsi));
> +	      gsi_remove (&gsi, false);
> +	    }
> +	  gimple *g;
> +	  basic_block bb = NULL;
> +	  edge e = NULL, ead = NULL;
> +	  FOR_EACH_VEC_ELT (arg_stmts, i, g)
> +	    {
> +	      gsi_safe_insert_before (&gsi, g);
> +	      if (i == 0)
> +		{
> +		  bb = gimple_bb (stmt);
> +		  gcc_checking_assert (EDGE_COUNT (bb->preds) == 2);
> +		  e = EDGE_PRED (bb, 0);
> +		  ead = EDGE_PRED (bb, 1);
> +		  if ((ead->flags & EDGE_ABNORMAL) == 0)
> +		    std::swap (e, ead);
> +		  gcc_checking_assert ((e->flags & EDGE_ABNORMAL) == 0
> +				       && (ead->flags & EDGE_ABNORMAL));
> +		}
> +	      tree lhs = gimple_assign_lhs (g);
> +	      tree arg = lhs;
> +	      gphi *phi = create_phi_node (copy_ssa_name (arg), bb);
> +	      add_phi_arg (phi, arg, e, UNKNOWN_LOCATION);
> +	      tree var = create_tmp_reg (TREE_TYPE (arg));
> +	      suppress_warning (var, OPT_Wuninitialized);
> +	      arg = get_or_create_ssa_default_def (cfun, var);
> +	      SSA_NAME_OCCURS_IN_ABNORMAL_PHI (arg) = 1;
> +	      add_phi_arg (phi, arg, ead, UNKNOWN_LOCATION);
> +	      arg = gimple_phi_result (phi);
> +	      SSA_NAME_OCCURS_IN_ABNORMAL_PHI (arg) = 1;
> +	      imm_use_iterator iter;
> +	      gimple *use_stmt;
> +	      FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
> +		{
> +		  if (use_stmt == phi)
> +		    continue;
> +		  gcc_checking_assert (use_stmt == stmt);
> +		  use_operand_p use_p;
> +		  FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
> +		    SET_USE (use_p, arg);
> +		}
> +	    }
> +	  update_stmt (stmt);
> +	  arg_stmts.truncate (0);
> +	}
> +    }
> +
>    return ret;
>  }
>  
> --- gcc/testsuite/gcc.dg/bitint-100.c.jj	2024-03-14 10:32:02.432644685 +0100
> +++ gcc/testsuite/gcc.dg/bitint-100.c	2024-03-14 11:21:16.444398599 +0100
> @@ -0,0 +1,108 @@
> +/* PR tree-optimization/113466 */
> +/* { dg-do compile { target bitint575 } } */
> +/* { dg-options "-O2" } */
> +
> +int foo (int);
> +
> +__attribute__((returns_twice, noipa)) _BitInt(325)
> +bar (_BitInt(575) x)
> +{
> +  (void) x;
> +  return 0wb;
> +}
> +
> +__attribute__((returns_twice, noipa)) _BitInt(325)
> +garply (_BitInt(575) x, _BitInt(575) y, _BitInt(575) z, int u, int v, _BitInt(575) w)
> +{
> +  (void) x;
> +  (void) y;
> +  (void) z;
> +  (void) u;
> +  (void) v;
> +  (void) w;
> +  return 0wb;
> +}
> +
> +_BitInt(325)
> +baz (_BitInt(575) y)
> +{
> +  foo (1);
> +  return bar (y);
> +}
> +
> +_BitInt(325)
> +qux (int x, _BitInt(575) y)
> +{
> +  if (x == 25)
> +    x = foo (2);
> +  else if (x == 42)
> +    x = foo (foo (3));
> +  return bar (y);
> +}
> +
> +void
> +corge (int x, _BitInt(575) y, _BitInt(325) *z)
> +{
> +  void *q[] = { &&l1, &&l2, &&l3, &&l3 };
> +  if (x == 25)
> +    {
> +    l1:
> +      x = foo (2);
> +    }
> +  else if (x == 42)
> +    {
> +    l2:
> +      x = foo (foo (3));
> +    }
> +l3:
> +  *z = bar (y);
> +  if (x < 4)
> +    goto *q[x & 3];
> +}
> +
> +_BitInt(325)
> +freddy (int x, _BitInt(575) y)
> +{
> +  bar (y);
> +  ++y;
> +  if (x == 25)
> +    x = foo (2);
> +  else if (x == 42)
> +    x = foo (foo (3));
> +  return bar (y);
> +}
> +
> +_BitInt(325)
> +quux (_BitInt(575) x, _BitInt(575) y, _BitInt(575) z)
> +{
> +  _BitInt(575) w = x + y;
> +  foo (1);
> +  return garply (x, y, z, 42, 42, w);
> +}
> +
> +_BitInt(325)
> +grault (int x, _BitInt(575) y, _BitInt(575) z)
> +{
> +  _BitInt(575) v = x + y;
> +  _BitInt(575) w = x - y;
> +  if (x == 25)
> +    x = foo (2);
> +  else if (x == 42)
> +    x = foo (foo (3));
> +  return garply (y, z, v, 0, 0, w);
> +}
> +
> +_BitInt(325)
> +plugh (int x, _BitInt(575) y, _BitInt(575) z, _BitInt(575) v, _BitInt(575) w)
> +{
> +  garply (y, z, v, 1, 2, w);
> +  ++y;
> +  z += 2wb;
> +  v <<= 3;
> +  w *= 3wb;
> +  if (x == 25)
> +    x = foo (2);
> +  else if (x == 42)
> +    x = foo (foo (3));
> +  return garply (y, z, v, 1, 2, w);
> +}
> 
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)