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. > -- > H.J.
