On December 14, 2014 3:08:28 PM CET, "H.J. Lu" <hjl.to...@gmail.com> wrote:
>On Mon, Dec 08, 2014 at 11:16:58PM +0100, Uros Bizjak wrote:
>> Hello!
>>
>> > On Tue, Nov 25, 2014 at 5:05 PM, H.J. Lu <hjl.to...@gmail.com>
>wrote:
>> >> On Tue, Nov 25, 2014 at 7:01 AM, Richard Biener
>> >> <richard.guent...@gmail.com> wrote:
>> >>> On Tue, Nov 25, 2014 at 1:57 PM, H.J. Lu <hongjiu...@intel.com>
>wrote:
>> >>>> Hi,
>> >>>>
>> >>>> The enclosed testcase fails on x86 when compiled with -Os since
>we pass
>> >>>> a byte parameter with a byte load in caller and read it as an
>int in
>> >>>> callee. The reason it only shows up with -Os is x86 backend
>encodes
>> >>>> a byte load with an int load if -O isn't used. When a byte load
>is
>> >>>> used, the upper 24 bits of the register have random value for
>none
>> >>>> WORD_REGISTER_OPERATIONS targets.
>> >>>>
>> >>>> It happens because setup_incoming_promotions in combine.c has
>> >>>>
>> >>>> /* The mode and signedness of the argument before any
>promotions happen
>> >>>> (equal to the mode of the pseudo holding it at that
>stage). */
>> >>>> mode1 = TYPE_MODE (TREE_TYPE (arg));
>> >>>> uns1 = TYPE_UNSIGNED (TREE_TYPE (arg));
>> >>>>
>> >>>> /* The mode and signedness of the argument after any
>source language and
>> >>>> TARGET_PROMOTE_PROTOTYPES-driven promotions. */
>> >>>> mode2 = TYPE_MODE (DECL_ARG_TYPE (arg));
>> >>>> uns3 = TYPE_UNSIGNED (DECL_ARG_TYPE (arg));
>> >>>>
>> >>>> /* The mode and signedness of the argument as it is
>actually passed,
>> >>>> after any TARGET_PROMOTE_FUNCTION_ARGS-driven ABI
>promotions. */
>> >>>> mode3 = promote_function_mode (DECL_ARG_TYPE (arg), mode2,
>&uns3,
>> >>>> TREE_TYPE (cfun->decl), 0);
>> >>>>
>> >>>> while they are actually passed in register by
>assign_parm_setup_reg in
>> >>>> function.c:
>> >>>>
>> >>>> /* Store the parm in a pseudoregister during the function, but
>we may
>> >>>> need to do it in a wider mode. Using 2 here makes the
>result
>> >>>> consistent with promote_decl_mode and thus
>expand_expr_real_1. */
>> >>>> promoted_nominal_mode
>> >>>> = promote_function_mode (data->nominal_type,
>data->nominal_mode, &unsignedp,
>> >>>> TREE_TYPE (current_function_decl),
>2);
>> >>>>
>> >>>> where nominal_type and nominal_mode are set up with TREE_TYPE
>(parm)
>> >>>> and TYPE_MODE (nominal_type). TREE_TYPE here is
>> >>>
>> >>> I think the bug is here, not in combine.c. Can you try going
>back in history
>> >>> for both snippets and see if they matched at some point?
>> >>>
>> >>
>> >> The bug was introduced by
>> >>
>> >> https://gcc.gnu.org/ml/gcc-cvs/2007-09/msg00613.html
>> >>
>> >> commit 5d93234932c3d8617ce92b77b7013ef6bede9508
>> >> Author: shinwell <shinwell@138bc75d-0d04-0410-961f-82ee72b054a4>
>> >> Date: Thu Sep 20 11:01:18 2007 +0000
>> >>
>> >> gcc/
>> >> * combine.c: Include cgraph.h.
>> >> (setup_incoming_promotions): Rework to allow more aggressive
>> >> elimination of sign extensions when all call sites of the
>> >> current function are known to lie within the current unit.
>> >>
>> >>
>> >> git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@128618
>> >> 138bc75d-0d04-0410-961f-82ee72b054a4
>> >>
>> >> Before this commit, combine.c has
>> >>
>> >> enum machine_mode mode = TYPE_MODE (TREE_TYPE (arg));
>> >> int uns = TYPE_UNSIGNED (TREE_TYPE (arg));
>> >>
>> >> mode = promote_mode (TREE_TYPE (arg), mode, &uns, 1);
>> >> if (mode == GET_MODE (reg) && mode != DECL_MODE (arg))
>> >> {
>> >> rtx x;
>> >> x = gen_rtx_CLOBBER (DECL_MODE (arg), const0_rtx);
>> >> x = gen_rtx_fmt_e ((uns ? ZERO_EXTEND :
>SIGN_EXTEND), mode, x);
>> >> record_value_for_reg (reg, first, x);
>> >> }
>> >>
>> >> It matches function.c:
>> >>
>> >> /* This is not really promoting for a call. However we need to
>be
>> >> consistent with assign_parm_find_data_types and
>expand_expr_real_1. */
>> >> promoted_nominal_mode
>> >> = promote_mode (data->nominal_type, data->nominal_mode,
>&unsignedp, 1);
>> >>
>> >> r128618 changed
>> >>
>> >> mode = promote_mode (TREE_TYPE (arg), mode, &uns, 1);
>> >>
>> >> to
>> >>
>> >> mode3 = promote_mode (DECL_ARG_TYPE (arg), mode2, &uns3, 1);
>> >>
>> >> It breaks none WORD_REGISTER_OPERATIONS targets.
>> >
>> > Hmm, I think that DECL_ARG_TYPE makes a difference only
>> > for non-WORD_REGISTER_OPERATIONS targets.
>> >
>> > But yeah, isolated the above change looks wrong.
>> >
>> > Your patch is ok for trunk if nobody objects within 24h and for
>branches
>> > after a week.
>> >
>> > Thanks,
>> > Richard.
>>
>> This patch caused PR64213.
>>
>
>Here is the updated patch. The difference is
>
> mode3 = promote_function_mode (TREE_TYPE (arg), mode1, &uns3,
> TREE_TYPE (cfun->decl), 0);
>
>vs
>
> mode3 = promote_function_mode (TREE_TYPE (arg), mode1, &uns1,
> TREE_TYPE (cfun->decl), 0);
>
>I made a mistake in my previous patch where I shouldn't have changed
>&uns3 to &uns1. We do want to update mode3 and uns3, not mode3 and
>uns1. It generates the same code on PR64213 testcase with a cross
>alpha-linux GCC.
>
>Uros, can you test it on Linux/alpha? OK for master, 4.9 and 4.8
>branches if it works on Linux/alpha?
OK for trunk and 4.9, please wait until after 4.8.4 is released for the 4.8
branch.
Thanks,
Richard.
>Thanks.
>
>
>H.J.
>---
>From 7b274d517dcaae96f111652283d947c035ab7a22 Mon Sep 17 00:00:00 2001
>From: "H.J. Lu" <hjl.to...@gmail.com>
>Date: Sun, 14 Dec 2014 05:37:41 -0800
>Subject: [PATCH] Pass unpromoted argument to promote_function_mode
>
>This patch updates setup_incoming_promotions in combine.c to match what
>is actually passed in assign_parm_setup_reg in function.c.
>
>gcc/
>
> PR rtl-optimization/64037
> * combine.c (setup_incoming_promotions): Pass the argument
> before any promotions happen to promote_function_mode.
>
>gcc/testsuite/
>
> PR rtl-optimization/64037
> * g++.dg/pr64037.C: New test.
>---
>diff --git a/gcc/combine.c b/gcc/combine.c
>index c95b493..ee7b3f9 100644
>--- a/gcc/combine.c
>+++ b/gcc/combine.c
>@@ -1579,8 +1579,8 @@ setup_incoming_promotions (rtx_insn *first)
> uns3 = TYPE_UNSIGNED (DECL_ARG_TYPE (arg));
>
> /* The mode and signedness of the argument as it is actually passed,
>- after any TARGET_PROMOTE_FUNCTION_ARGS-driven ABI promotions.
> */
>- mode3 = promote_function_mode (DECL_ARG_TYPE (arg), mode2,
>&uns3,
>+ see assign_parm_setup_reg in function.c. */
>+ mode3 = promote_function_mode (TREE_TYPE (arg), mode1, &uns3,
> TREE_TYPE (cfun->decl), 0);
>
> /* The mode of the register in which the argument is being passed. */
>diff --git a/gcc/testsuite/g++.dg/pr64037.C
>b/gcc/testsuite/g++.dg/pr64037.C
>new file mode 100644
>index 0000000..e5cd0e2
>--- /dev/null
>+++ b/gcc/testsuite/g++.dg/pr64037.C
>@@ -0,0 +1,27 @@
>+// { dg-do run { target i?86-*-* x86_64-*-* } }
>+// { dg-options "-std=c++11 -Os" }
>+
>+enum class X : unsigned char {
>+ V = 2,
>+};
>+
>+static void
>+__attribute__((noinline,noclone))
>+foo(unsigned &out, unsigned a, X b)
>+{
>+ out = static_cast<unsigned>(b);
>+}
>+
>+int main()
>+{
>+ unsigned deadbeef = 0xDEADBEEF;
>+ asm volatile ("" : "+d" (deadbeef), "+c" (deadbeef));
>+
>+ unsigned out;
>+ foo(out, 2, X::V);
>+
>+ if (out != 2)
>+ __builtin_abort ();
>+
>+ return 0;
>+}
