[PATCH 2/9] ifcvt: Use enum instead of transform_name string.

Richard Sandiford richard.sandiford@arm.com
Tue Aug 6 21:03:00 GMT 2019


Robin Dapp <rdapp@linux.ibm.com> writes:
> This patch introduces an enum for ifcvt's various noce transformations.
> As the transformation might be queried by the backend, I find it nicer
> to allow checking for a proper type instead of a string comparison.

TBH I think the names of internal ifcvt routines are too level to expose as
an "official" part of the interface.  (OK, like you say, the information's
already there and rogue backends could already check it if they wanted.)

If we were going to have an enum, I think it should be a higher-level
description of what the converted code is going to do, rather than the
name of the routine that's going to do it.

It sounded from the covering note that you might not need this
any more though.

Thanks,
Richard

>
> ---
>  gcc/ifcvt.c | 46 ++++++++++++++++++------------------
>  gcc/ifcvt.h | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 88 insertions(+), 25 deletions(-)
>
> diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
> index b80dbc43d83..e95ff9ee9b0 100644
> --- a/gcc/ifcvt.c
> +++ b/gcc/ifcvt.c
> @@ -1105,7 +1105,7 @@ noce_try_move (struct noce_if_info *if_info)
>  	  emit_insn_before_setloc (seq, if_info->jump,
>  				   INSN_LOCATION (if_info->insn_a));
>  	}
> -      if_info->transform_name = "noce_try_move";
> +      if_info->transform = ifcvt_transform_noce_try_move;
>        return TRUE;
>      }
>    return FALSE;
> @@ -1139,7 +1139,7 @@ noce_try_ifelse_collapse (struct noce_if_info * if_info)
>    emit_insn_before_setloc (seq, if_info->jump,
>  			  INSN_LOCATION (if_info->insn_a));
>  
> -  if_info->transform_name = "noce_try_ifelse_collapse";
> +  if_info->transform = ifcvt_transform_noce_try_ifelse_collapse;
>    return TRUE;
>  }
>  
> @@ -1186,7 +1186,7 @@ noce_try_store_flag (struct noce_if_info *if_info)
>  
>        emit_insn_before_setloc (seq, if_info->jump,
>  			       INSN_LOCATION (if_info->insn_a));
> -      if_info->transform_name = "noce_try_store_flag";
> +      if_info->transform = ifcvt_transform_noce_try_store_flag;
>        return TRUE;
>      }
>    else
> @@ -1265,7 +1265,7 @@ noce_try_inverse_constants (struct noce_if_info *if_info)
>  
>        emit_insn_before_setloc (seq, if_info->jump,
>  			       INSN_LOCATION (if_info->insn_a));
> -      if_info->transform_name = "noce_try_inverse_constants";
> +      if_info->transform = ifcvt_transform_noce_try_inverse_constants;
>        return true;
>      }
>  
> @@ -1485,7 +1485,7 @@ noce_try_store_flag_constants (struct noce_if_info *if_info)
>  
>        emit_insn_before_setloc (seq, if_info->jump,
>  			       INSN_LOCATION (if_info->insn_a));
> -      if_info->transform_name = "noce_try_store_flag_constants";
> +      if_info->transform = ifcvt_transform_noce_try_store_flag_constants;
>  
>        return TRUE;
>      }
> @@ -1546,7 +1546,7 @@ noce_try_addcc (struct noce_if_info *if_info)
>  
>  	      emit_insn_before_setloc (seq, if_info->jump,
>  				       INSN_LOCATION (if_info->insn_a));
> -	      if_info->transform_name = "noce_try_addcc";
> +	      if_info->transform = ifcvt_transform_noce_try_addcc;
>  
>  	      return TRUE;
>  	    }
> @@ -1588,7 +1588,7 @@ noce_try_addcc (struct noce_if_info *if_info)
>  
>  	      emit_insn_before_setloc (seq, if_info->jump,
>  				       INSN_LOCATION (if_info->insn_a));
> -	      if_info->transform_name = "noce_try_addcc";
> +	      if_info->transform = ifcvt_transform_noce_try_addcc;
>  	      return TRUE;
>  	    }
>  	  end_sequence ();
> @@ -1639,7 +1639,7 @@ noce_try_store_flag_mask (struct noce_if_info *if_info)
>  
>  	  emit_insn_before_setloc (seq, if_info->jump,
>  				   INSN_LOCATION (if_info->insn_a));
> -	  if_info->transform_name = "noce_try_store_flag_mask";
> +	  if_info->transform = ifcvt_transform_noce_try_store_flag_mask;
>  
>  	  return TRUE;
>  	}
> @@ -1791,7 +1791,7 @@ noce_try_cmove (struct noce_if_info *if_info)
>  
>  	  emit_insn_before_setloc (seq, if_info->jump,
>  				   INSN_LOCATION (if_info->insn_a));
> -	  if_info->transform_name = "noce_try_cmove";
> +	  if_info->transform = ifcvt_transform_noce_try_cmove;
>  
>  	  return TRUE;
>  	}
> @@ -1844,7 +1844,7 @@ noce_try_cmove (struct noce_if_info *if_info)
>  
>  	      emit_insn_before_setloc (seq, if_info->jump,
>  				   INSN_LOCATION (if_info->insn_a));
> -	      if_info->transform_name = "noce_try_cmove";
> +	      if_info->transform = ifcvt_transform_noce_try_cmove;
>  	      return TRUE;
>  	    }
>  	  else
> @@ -2286,7 +2286,7 @@ noce_try_cmove_arith (struct noce_if_info *if_info)
>  
>    emit_insn_before_setloc (ifcvt_seq, if_info->jump,
>  			   INSN_LOCATION (if_info->insn_a));
> -  if_info->transform_name = "noce_try_cmove_arith";
> +  if_info->transform = ifcvt_transform_noce_try_cmove_arith;
>    return TRUE;
>  
>   end_seq_and_fail:
> @@ -2544,7 +2544,7 @@ noce_try_minmax (struct noce_if_info *if_info)
>    if_info->cond = cond;
>    if_info->cond_earliest = earliest;
>    if_info->rev_cond = NULL_RTX;
> -  if_info->transform_name = "noce_try_minmax";
> +  if_info->transform = ifcvt_transform_noce_try_minmax;
>  
>    return TRUE;
>  }
> @@ -2712,7 +2712,7 @@ noce_try_abs (struct noce_if_info *if_info)
>    if_info->cond = cond;
>    if_info->cond_earliest = earliest;
>    if_info->rev_cond = NULL_RTX;
> -  if_info->transform_name = "noce_try_abs";
> +  if_info->transform = ifcvt_transform_noce_try_abs;
>  
>    return TRUE;
>  }
> @@ -2794,7 +2794,7 @@ noce_try_sign_mask (struct noce_if_info *if_info)
>      return FALSE;
>  
>    emit_insn_before_setloc (seq, if_info->jump, INSN_LOCATION (if_info->insn_a));
> -  if_info->transform_name = "noce_try_sign_mask";
> +  if_info->transform = ifcvt_transform_noce_try_sign_mask;
>  
>    return TRUE;
>  }
> @@ -2904,7 +2904,7 @@ noce_try_bitop (struct noce_if_info *if_info)
>        emit_insn_before_setloc (seq, if_info->jump,
>  			       INSN_LOCATION (if_info->insn_a));
>      }
> -  if_info->transform_name = "noce_try_bitop";
> +  if_info->transform = ifcvt_transform_noce_try_bitop;
>    return TRUE;
>  }
>  
> @@ -3244,16 +3244,17 @@ noce_convert_multiple_sets (struct noce_if_info *if_info)
>    for (int i = 0; i < count; i++)
>      noce_emit_move_insn (targets[i], temporaries[i]);
>  
> +
>    /* Actually emit the sequence if it isn't too expensive.  */
>    rtx_insn *seq = get_insns ();
>  
> -  if_info->transform_name = "noce_convert_multiple_sets";
> +  if_info->transform = ifcvt_transform_noce_convert_multiple_sets;
>    if_info->created_cmovs = count;
>  
>    if (!targetm.noce_conversion_profitable_p (seq, if_info))
>      {
>        end_sequence ();
> -      if_info->transform_name = "";
> +      if_info->transform = ifcvt_transform_none;
>        if_info->created_cmovs = 0;
>        return FALSE;
>      }
> @@ -3400,15 +3401,16 @@ noce_process_if_block (struct noce_if_info *if_info)
>      {
>        if (noce_convert_multiple_sets (if_info))
>  	{
> -	  if (dump_file && if_info->transform_name)
> +	  if (dump_file && if_info->transform)
>  	    fprintf (dump_file, "if-conversion succeeded through %s\n",
> -		     if_info->transform_name);
> +		ifcvt_get_transform_name (if_info->transform));
>  	  return TRUE;
>  	}
>      }
>  
>    bool speed_p = optimize_bb_for_speed_p (test_bb);
>    unsigned int then_cost = 0, else_cost = 0;
> +
>    if (!bb_valid_for_noce_process_p (then_bb, cond, &then_cost,
>  				    &if_info->then_simple))
>      return false;
> @@ -3619,9 +3621,9 @@ noce_process_if_block (struct noce_if_info *if_info)
>    return FALSE;
>  
>   success:
> -  if (dump_file && if_info->transform_name)
> +  if (dump_file && if_info->transform)
>      fprintf (dump_file, "if-conversion succeeded through %s\n",
> -	     if_info->transform_name);
> +	ifcvt_get_transform_name (if_info->transform));
>  
>    /* If we used a temporary, fix it up now.  */
>    if (orig_x != x)
> @@ -4065,7 +4067,7 @@ noce_find_if_block (basic_block test_bb, edge then_edge, edge else_edge,
>       and jump_insns are always given a cost of 1 by seq_cost, so treat
>       both instructions as having cost COSTS_N_INSNS (1).  */
>    if_info.original_cost = COSTS_N_INSNS (2);
> -  if_info.transform_name = "";
> +  if_info.transform = ifcvt_transform_none;
>    if_info.created_cmovs = 0;
>  
>    /* Do the real work.  */
> diff --git a/gcc/ifcvt.h b/gcc/ifcvt.h
> index 130559c0911..3776a25c3f6 100644
> --- a/gcc/ifcvt.h
> +++ b/gcc/ifcvt.h
> @@ -40,6 +40,66 @@ struct ce_if_block
>    int pass;				/* Pass number.  */
>  };
>  
> +enum ifcvt_transform
> +{
> +  ifcvt_transform_none = 0,
> +  ifcvt_transform_noce_try_move,
> +  ifcvt_transform_noce_try_ifelse_collapse,
> +  ifcvt_transform_noce_try_store_flag,
> +  ifcvt_transform_noce_try_inverse_constants,
> +  ifcvt_transform_noce_try_store_flag_constants,
> +  ifcvt_transform_noce_try_addcc,
> +  ifcvt_transform_noce_try_store_flag_mask,
> +  ifcvt_transform_noce_try_cmove,
> +  ifcvt_transform_noce_try_cmove_arith,
> +  ifcvt_transform_noce_try_minmax,
> +  ifcvt_transform_noce_try_abs,
> +  ifcvt_transform_noce_try_sign_mask,
> +  ifcvt_transform_noce_try_bitop,
> +  ifcvt_transform_noce_convert_multiple_sets
> +};
> +
> +inline const char *
> +ifcvt_get_transform_name (enum ifcvt_transform transform)
> +{
> +  switch (transform)
> +    {
> +    case ifcvt_transform_none:
> +      return "";
> +    case ifcvt_transform_noce_try_move:
> +      return "noce_try_move";
> +    case ifcvt_transform_noce_try_ifelse_collapse:
> +      return "noce_try_ifelse_collapse";
> +    case ifcvt_transform_noce_try_store_flag:
> +      return "noce_try_store_flag";
> +    case ifcvt_transform_noce_try_inverse_constants:
> +      return "noce_try_inverse_constants";
> +    case ifcvt_transform_noce_try_store_flag_constants:
> +      return "noce_try_store_flag_constants";
> +    case ifcvt_transform_noce_try_addcc:
> +      return "noce_try_addcc";
> +    case ifcvt_transform_noce_try_store_flag_mask:
> +      return "noce_try_store_flag_mask";
> +    case ifcvt_transform_noce_try_cmove:
> +      return "noce_try_cmove";
> +    case ifcvt_transform_noce_try_cmove_arith:
> +      return "noce_try_cmove_arith";
> +    case ifcvt_transform_noce_try_minmax:
> +      return "noce_try_minmax";
> +    case ifcvt_transform_noce_try_abs:
> +      return "noce_try_abs";
> +    case ifcvt_transform_noce_try_sign_mask:
> +      return "noce_try_sign_mask";
> +    case ifcvt_transform_noce_try_bitop:
> +      return "noce_try_bitop";
> +    case ifcvt_transform_noce_convert_multiple_sets:
> +      return "noce_convert_multiple_sets";
> +    default:
> +      return "unknown transform";
> +    }
> +}
> +
> +
>  /* Used by noce_process_if_block to communicate with its subroutines.
>  
>     The subroutines know that A and B may be evaluated freely.  They
> @@ -105,9 +165,10 @@ struct noce_if_info
>       generate to replace this branch.  */
>    unsigned int max_seq_cost;
>  
> -  /* The name of the noce transform that succeeded in if-converting
> -     this structure.  Used for debugging.  */
> -  const char *transform_name;
> +  /* The noce transform that succeeded in if-converting this structure.
> +     Used for letting the backend determine which transform succeeded
> +     and for debugging output.  */
> +  enum ifcvt_transform transform;
>  
>    /* The number of created conditional moves in case we convert multiple
>       sets.  */



More information about the Gcc-patches mailing list