Re: [Gfortran, Patch] PR12456 - Optimize single character.

[Andrew Pinski <pinskia at physics dot uc dot edu>]

On Jan 3, 2006, at 11:40 PM, Feng Wang wrote:

> Index: trans-expr.c
> ===================================================================
>
> +      /* Deal with single character specially.  */
> +      if (sc1 != NULL_TREE && sc2 != NULL_TREE)
> +	{
> +	  lse.expr = build2 (MINUS_EXPR, gfc_get_int_type (4), sc1, sc2);
> +	}

This is wrong with respect with types.
You want to do something like:
lse.expr = fold_convert (gfc_get_int_type (4),
					 fold_build2 (MINUS_EXPR, gfc_character1_type_node,
								sc1, sc2));

This will get the types correctly but this does the subtraction in the 
character
type which is unsigned. Hmm, Maybe it should look like this changing 
the type of
the characters to signed integers and then doing the subtraction in 
that type:
{
  tree type = gfc_get_int_type (4)
  sc1 = fold_convert (type, sc1);
  sc2 = fold_convert (type, sc2);

  lse.expr = fold_build2 (MINUS_EXPR, type, sc1, sc2);
}

> @@ -2545,6 +2570,22 @@ gfc_conv_string_parameter (gfc_se * se)
>  	  && TREE_CODE (TREE_TYPE (se->string_length)) == INTEGER_TYPE);
>  }
>
> +/* If a string's length is one, we convert it to a single character.  
> */
> +
> +tree
> +gfc_to_single_character (tree str, tree len)
> +{
> +  gcc_assert (POINTER_TYPE_P (TREE_TYPE (str)));
> +
> +  if (INTEGER_CST_P (len) && TREE_INT_CST_LOW (len) == 1
> +    && TREE_INT_CST_HIGH (len) == 0)
> +    {
> +      str = build1 (NOP_EXPR, pchar_type_node, str);

Since we know that this is a pointer already, can you use
"fold_convert (pchar_type_node, str);" instead of "build1 (NOP_EXPR"?
This will remove an extra NOP_EXPR if we don't need it and
also try to fold it if we can.

> Index: trans-intrinsic.c
> ===================================================================
> --- trans-intrinsic.c	(revision 109308)
> +++ trans-intrinsic.c	(working copy)
> @@ -2267,10 +2267,27 @@ gfc_conv_intrinsic_strcmp (gfc_se * se,
>  {
....
> +  if (arg1 != NULL_TREE && arg2 != NULL_TREE)
> +    {
> +      se->expr = build2 (MINUS_EXPR, gfc_get_int_type (4), arg1, 
> arg2);
> +    }
> +  else
> +    {
> +      /* Build a call for the comparison.  */
> +      se->expr = build_function_call_expr 
> (gfor_fndecl_compare_string, args);
> +    }
>
>    type = gfc_typenode_for_spec (&expr->ts);
>    se->expr = build2 (op, type, se->expr,
The same issue as gfc_conv_expr_op applies here also.

Everything else looks good.  Can you retest with my suggestions and 
repost
it?

I should note for some cleanup; it would be better if there is a 
function
which takes the common parts of gfc_conv_intrinsic_strcmp and 
gfc_conv_expr_op
since they do similar things. But that is for a different time.

Thanks,
Andrew Pinski