From: Marek Polacek <polacek@redhat.com>
To: GCC Patches <gcc-patches@gcc.gnu.org>, Jason Merrill <jason@redhat.com>
Subject: [c++-delayed-folding] First stab at convert_to_integer
Date: Fri, 16 Oct 2015 17:42:00 -0000 [thread overview]
Message-ID: <20151016173541.GN13672@redhat.com> (raw)
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
next reply other threads:[~2015-10-16 17:35 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-16 17:42 Marek Polacek [this message]
2015-10-17 0:25 ` Jason Merrill
2015-10-19 12:38 ` Marek Polacek
2015-10-19 20:02 ` Jason Merrill
2015-10-19 20:23 ` Marek Polacek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151016173541.GN13672@redhat.com \
--to=polacek@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jason@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).