Hi!
On Mon, Mar 27, 2017 at 09:51:27AM -0600, Jeff Law wrote:
> > 2017-03-24 Jakub Jelinek <[email protected]>
> >
> > PR c/80163
> > * expr.c <CASE_CONVERT>: For EXPAND_INITIALIZER determine SIGN_EXTEND
> > vs. ZERO_EXTEND based on signedness of treeop0's type rather than
> > signedness of the result type.
> >
> > * gcc.dg/torture/pr80163.c: New test.
> >
> > --- gcc/expr.c.jj 2017-03-07 09:04:04.000000000 +0100
> > +++ gcc/expr.c 2017-03-24 12:09:57.729854079 +0100
> > @@ -8333,7 +8333,8 @@ expand_expr_real_2 (sepops ops, rtx targ
> > }
> >
> > else if (modifier == EXPAND_INITIALIZER)
> > - op0 = gen_rtx_fmt_e (unsignedp ? ZERO_EXTEND : SIGN_EXTEND, mode, op0);
> > + op0 = gen_rtx_fmt_e (TYPE_UNSIGNED (TREE_TYPE (treeop0))
> > + ? ZERO_EXTEND : SIGN_EXTEND, mode, op0);
> ?!?
>
> Shouldn't the zero/sign extension be derived from the target's type not the
> source types?
No, it needs to be derived from the source operand type, that is exactly the
bug here, we were deriving it from target's type.
> treeop0 is the first source operand, isn't it?
Yes.
If you look at the surrounding code, you can see e.g.:
else if (target == 0)
op0 = convert_to_mode (mode, op0,
TYPE_UNSIGNED (TREE_TYPE
(treeop0)));
else
{
convert_move (target, op0,
TYPE_UNSIGNED (TREE_TYPE (treeop0)));
op0 = target;
}
where the conversion is derived from the operand type, or also:
else if (CONSTANT_P (op0))
{
tree inner_type = TREE_TYPE (treeop0);
machine_mode inner_mode = GET_MODE (op0);
if (inner_mode == VOIDmode)
inner_mode = TYPE_MODE (inner_type);
if (modifier == EXPAND_INITIALIZER)
op0 = lowpart_subreg (mode, op0, inner_mode);
else
op0= convert_modes (mode, inner_mode, op0,
TYPE_UNSIGNED (inner_type));
}
(the lowpart_subreg unconditionally looks problematic too, e.g. if
op0 happens to be scalar int and mode is integral; but let's
deal with it incrementally; perhaps we are always folded and never
reach here with CONST_INTs).
Or consider what kind of extension you need if you have conversions
int -> unsigned long long SIGN_EXTEND
unsigned int -> signed long long ZERO_EXTEND
(and just for completeness):
int -> signed long long (obviously SIGN_EXTEND)
unsigned int -> unsigned long long (obviously ZERO_EXTEND)
Jakub
