This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[c++-delayed-folding] First stab at convert_to_integer


I felt like it'd be good to resolve some unfinished business in
convert_to_integer before starting messing with other convert_to_*
functions.  This is a response to
<https://gcc.gnu.org/ml/gcc-patches/2015-09/msg01255.html>.

> > +      if (!dofold)
> > +        {
> > +         expr = build1 (CONVERT_EXPR,
> > +                        lang_hooks.types.type_for_size
> > +                          (TYPE_PRECISION (intype), 0),
> > +                        expr);
> > +         return build1 (CONVERT_EXPR, type, expr);
> > +       }
>
> When we're not folding, I don't think we want to do the two-step 
> conversion, just the second one.  And we might want to use NOP_EXPR 
> instead of CONVERT_EXPR, but I'm not sure about that.

Changed.  I kept CONVERT_EXPR there even though NOP_EXPR seemed to work as
well.

> > @@ -818,10 +828,15 @@ convert_to_integer (tree type, tree expr)
> >                         if (TYPE_UNSIGNED (typex))
> >                           typex = signed_type_for (typex);
> >                       }
> > -                   return convert (type,
> > -                                   fold_build2 (ex_form, typex,
> > -                                                convert (typex, arg0),
> > -                                                convert (typex, arg1)));
> > +                   if (dofold)
> > +                     return convert (type,
> > +                                     fold_build2 (ex_form, typex,
> > +                                                  convert (typex, arg0),
> > +                                                  convert (typex, arg1)));
> > +                   arg0 = build1 (CONVERT_EXPR, typex, arg0);
> > +                   arg1 = build1 (CONVERT_EXPR, typex, arg1);
> > +                   expr = build2 (ex_form, typex, arg0, arg1);
> > +                   return build1 (CONVERT_EXPR, type, expr);
>
> This code path seems to be for pushing a conversion down into a binary 
> expression.  We shouldn't do this at all when we aren't folding.

I tend to agree, but this case is tricky.  What's this code about is
e.g. for

int
fn (long p, long o)
{
  return p + o;
}

we want to narrow the operation and do the addition on unsigned ints and then
convert to int.  We do it here because we're still missing the
promotion/demotion pass on GIMPLE (PR45397 / PR47477).  Disabling this
optimization here would regress a few testcases, so I kept the code as it was.
Thoughts?

> > @@ -845,9 +860,14 @@ convert_to_integer (tree type, tree expr)
> >
> >             if (!TYPE_UNSIGNED (typex))
> >               typex = unsigned_type_for (typex);
> > +           if (!dofold)
> > +             return build1 (CONVERT_EXPR, type,
> > +                            build1 (ex_form, typex,
> > +                                    build1 (CONVERT_EXPR, typex,
> > +                                            TREE_OPERAND (expr, 0))));
>
> Likewise.

Changed.

> > @@ -867,6 +887,14 @@ convert_to_integer (tree type, tree expr)
> >              the conditional and never loses.  A COND_EXPR may have a throw
> >              as one operand, which then has void type.  Just leave void
> >              operands as they are.  */
> > +         if (!dofold)
> > +           return build3 (COND_EXPR, type, TREE_OPERAND (expr, 0),
> > +                          VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (expr, 1)))
> > +                          ? TREE_OPERAND (expr, 1)
> > +                          : build1 (CONVERT_EXPR, type, TREE_OPERAND (expr, 1)),
> > +                          VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (expr, 2)))
> > +                          ? TREE_OPERAND (expr, 2)
> > +                          : build1 (CONVERT_EXPR, type, TREE_OPERAND (expr, 2)));
>
> Likewise.

Changed.

> > @@ -903,6 +933,10 @@ convert_to_integer (tree type, tree expr)
> >        return build1 (FIXED_CONVERT_EXPR, type, expr);
> >
> >      case COMPLEX_TYPE:
> > +      if (!dofold)
> > +       return build1 (CONVERT_EXPR, type,
> > +                      build1 (REALPART_EXPR,
> > +                              TREE_TYPE (TREE_TYPE (expr)), expr));
>
> Why can't we call convert here rather than build1 a CONVERT_EXPR?

I don't know if there was any particular reason but just build1 seems dubious
so I've changed this to convert and didn't see any problems.

> It would be good to ask a fold/convert maintainer to review the changes 
> to this file, too.

Certainly; comments welcome.

Moreover, there are some places in the C++ FE where we still call
convert_to_integer and not convert_to_integer_nofold -- should they be
changed to the _nofold variant?

Bootstrapped/regtested on x86_64-linux, ok for branch?

diff --git gcc/convert.c gcc/convert.c
index fdb9b9a..40db767 100644
--- gcc/convert.c
+++ gcc/convert.c
@@ -571,13 +571,7 @@ convert_to_integer_1 (tree type, tree expr, bool dofold)
 	 coexistence of multiple valid pointer sizes, so fetch the one we need
 	 from the type.  */
       if (!dofold)
-        {
-	  expr = build1 (CONVERT_EXPR,
-			 lang_hooks.types.type_for_size
-			   (TYPE_PRECISION (intype), 0),
-			 expr);
-	  return build1 (CONVERT_EXPR, type, expr);
-	}
+	return build1 (CONVERT_EXPR, type, expr);
       expr = fold_build1 (CONVERT_EXPR,
 			  lang_hooks.types.type_for_size
 			    (TYPE_PRECISION (intype), 0),
@@ -622,6 +616,8 @@ convert_to_integer_1 (tree type, tree expr, bool dofold)
 	  else
 	    code = NOP_EXPR;
 
+	  if (!dofold)
+	    return build1 (code, type, expr);
 	  return fold_build1 (code, type, expr);
 	}
 
@@ -847,6 +843,9 @@ convert_to_integer_1 (tree type, tree expr, bool dofold)
 	  /* This is not correct for ABS_EXPR,
 	     since we must test the sign before truncation.  */
 	  {
+	    if (!dofold)
+	      break;
+
 	    /* Do the arithmetic in type TYPEX,
 	       then convert result to TYPE.  */
 	    tree typex = type;
@@ -860,11 +859,6 @@ convert_to_integer_1 (tree type, tree expr, bool dofold)
 
 	    if (!TYPE_UNSIGNED (typex))
 	      typex = unsigned_type_for (typex);
-	    if (!dofold)
-	      return build1 (CONVERT_EXPR, type,
-			     build1 (ex_form, typex,
-				     build1 (CONVERT_EXPR, typex,
-					     TREE_OPERAND (expr, 0))));
 	    return convert (type,
 			    fold_build1 (ex_form, typex,
 					  convert (typex,
@@ -887,21 +881,15 @@ convert_to_integer_1 (tree type, tree expr, bool dofold)
 	     the conditional and never loses.  A COND_EXPR may have a throw
 	     as one operand, which then has void type.  Just leave void
 	     operands as they are.  */
-	  if (!dofold)
-	    return build3 (COND_EXPR, type, TREE_OPERAND (expr, 0),
+	  if (dofold)
+	    return
+	      fold_build3 (COND_EXPR, type, TREE_OPERAND (expr, 0),
 			   VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (expr, 1)))
 			   ? TREE_OPERAND (expr, 1)
-			   : build1 (CONVERT_EXPR, type, TREE_OPERAND (expr, 1)),
+			   : convert (type, TREE_OPERAND (expr, 1)),
 			   VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (expr, 2)))
 			   ? TREE_OPERAND (expr, 2)
-			   : build1 (CONVERT_EXPR, type, TREE_OPERAND (expr, 2)));
-	  return fold_build3 (COND_EXPR, type, TREE_OPERAND (expr, 0),
-			      VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (expr, 1)))
-			      ? TREE_OPERAND (expr, 1)
-			      : convert (type, TREE_OPERAND (expr, 1)),
-			      VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (expr, 2)))
-			      ? TREE_OPERAND (expr, 2)
-			      : convert (type, TREE_OPERAND (expr, 2)));
+			   : convert (type, TREE_OPERAND (expr, 2)));
 
 	default:
 	  break;
@@ -934,9 +922,9 @@ convert_to_integer_1 (tree type, tree expr, bool dofold)
 
     case COMPLEX_TYPE:
       if (!dofold)
-	return build1 (CONVERT_EXPR, type,
-		       build1 (REALPART_EXPR,
-			       TREE_TYPE (TREE_TYPE (expr)), expr));
+	return convert (type,
+			build1 (REALPART_EXPR,
+				TREE_TYPE (TREE_TYPE (expr)), expr));
       return convert (type,
 		      fold_build1 (REALPART_EXPR,
 				   TREE_TYPE (TREE_TYPE (expr)), expr));

	Marek


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]