On 03/06/2013 09:43 AM, Jakub Jelinek wrote:
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 <ja...@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.
Doesn't the code in update_accumulator_with_ops need the same change?
Unrelated, but the block comment still refers to UPDATE, which is no
longer a parameter.
I see similar looking code in tree-inline.c::copy_bb... Does it need
updating as well?
Change looks OK to me, but please check those other two instances and
take appropriate action. If you could update the block comment before
adjust_return_value_with_ops it'd be appreciated as well.
jeff
