[Gfortran, Patch] PR12456 - Optimize single character.

[Feng Wang]

--- Andrew Pinski <pinskia@physics.uc.edu> wrote:
> 
> 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);
> }

Yes, you are right. The type of character is unsigned char. We should first
convert to singned integer.

Paul said the use of gfc_get_int_type (4) is wrong. I agree. We should avoid
using anything "machine" dependent when dealing with language layer items. What
about gfc_get_int_type (gfc_default_integer_kind)? Paul, could you explain
more?

> 
> > @@ -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.

OK.

> 
> > 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.
> 

I will try.


Best Regards,
Feng Wang

--
Creative Compiler Research Group,
National University of Defense Technology, China.


		
___________________________________________________________ 
ÑÅ»¢1GÃâ·ÑÓÊÏä°Ù·Ö°Ù·ÀÀ¬»øÐÅ 
http://cn.mail.yahoo.com