Hi!

On Mon, Mar 27, 2017 at 09:51:27AM -0600, Jeff Law wrote:
> > 2017-03-24  Jakub Jelinek  <ja...@redhat.com>
> > 
> >     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

Reply via email to