public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Richard Biener <rguenther@suse.de>
Cc: gcc-patches@gcc.gnu.org
Subject: [PATCH] Fix up adjust_return_value_with_ops (PR tree-optimization/56539)
Date: Wed, 06 Mar 2013 16:43:00 -0000	[thread overview]
Message-ID: <20130306164339.GX12913@tucnak.redhat.com> (raw)

Hi!

This patch fixes a bug where force_gimple_operand_gsi was called with
bogus last argument.  The before argument is true, and the stmt that is
added at the end of course should be put in between the (optional) sequence
added by force_gimple_operand_gsi and gsi_stmt (gsi), so it should use
GSI_SAME_STMT, otherwise we end up using SSA_NAME in stmt defined later on
within the same bb.

Bootstrapped on i686-linux, bootstrap on x86_64-linux and both regtests
still pending, ok for trunk if it succeeds?

BTW, I think we still have a latent issue there, because in the testcase
the computations are done in unsigned short type (both multiplications
and additions), so they never cause undefined behavior on overflow, but
in what tail recursion generates, the updates on a_acc and m_acc look fine
(done in unsigned short type), the accumulators themselves are signed short
(in theory fine too), but at the end the additions and multiplications
with a_acc/m_acc are done in signed short type.  I guess I should open a PR
for that.  E.g. for the simpler:
short foo (const char *x, unsigned y) { return y > 1 ? 10 * foo (x, y - 1) : (*x - '0'); }
there is signed short multiplication at the end rather than unsigned already
in GCC 4.4.

2013-03-06  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/56539
	* tree-tailcall.c (adjust_return_value_with_ops): Use GSI_SAME_STMT
	instead of GSI_CONTINUE_LINKING as last argument to
	force_gimple_operand_gsi.

	* gcc.c-torture/compile/pr56539.c: New test.

--- gcc/tree-tailcall.c.jj	2013-02-08 13:16:55.000000000 +0100
+++ gcc/tree-tailcall.c	2013-03-06 15:02:35.836468072 +0100
@@ -622,7 +622,7 @@ adjust_return_value_with_ops (enum tree_
 					    fold_convert (TREE_TYPE (op1), acc),
 					    op1));
       rhs = force_gimple_operand_gsi (&gsi, rhs,
-				      false, NULL, true, GSI_CONTINUE_LINKING);
+				      false, NULL, true, GSI_SAME_STMT);
       stmt = gimple_build_assign (result, rhs);
     }
 
--- gcc/testsuite/gcc.c-torture/compile/pr56539.c.jj	2013-03-06 15:05:50.696330194 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr56539.c	2013-03-06 15:05:32.000000000 +0100
@@ -0,0 +1,7 @@
+/* PR tree-optimization/56539 */
+
+short
+foo (const char *x, unsigned y)
+{
+  return y > 1 ? (x[y - 1] - '0') + 10 * foo (x, y - 1) : (*x - '0');
+}

	Jakub

             reply	other threads:[~2013-03-06 16:43 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-06 16:43 Jakub Jelinek [this message]
2013-03-06 22:00 ` Jeff Law
2013-03-06 23:28   ` Jakub Jelinek
2013-03-07 16:38     ` Jeff Law

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=20130306164339.GX12913@tucnak.redhat.com \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rguenther@suse.de \
    /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).