* [PATCH] Fix expansion of initializer extensions (PR c/80163)
@ 2017-03-24 19:00 Jakub Jelinek
2017-03-27 15:52 ` Jeff Law
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2017-03-24 19:00 UTC (permalink / raw)
To: Jeff Law, Bernd Schmidt; +Cc: gcc-patches
Hi!
As can be seen on the following testcase, when expanding an extension
in EXPAND_INITIALIZER context, we emit wrong extension operation
(one depending on the signedness of the result type rather than
on the signedness of the argument type, so e.g. extension of
unsigned int to long long int is done using SIGN_EXTEND instead of
ZERO_EXTEND, and extension of int to unsigned long long int using
ZERO_EXTEND instead of SIGN_EXTEND.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
ok for trunk?
2017-03-24 Jakub Jelinek <jakub@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);
else if (target == 0)
op0 = convert_to_mode (mode, op0,
--- gcc/testsuite/gcc.dg/torture/pr80163.c.jj 2017-03-24 12:19:21.363603066 +0100
+++ gcc/testsuite/gcc.dg/torture/pr80163.c 2017-03-24 12:19:09.000000000 +0100
@@ -0,0 +1,35 @@
+/* PR c/80163 */
+/* { dg-do compile { target int128 } } */
+
+volatile int v;
+
+__attribute__((noinline, noclone)) void
+bar (void)
+{
+ v++;
+ asm volatile ("" : : : "memory");
+}
+
+__attribute__((noinline, noclone)) __int128_t *
+foo (unsigned long **p)
+{
+a:
+ bar ();
+b:
+ bar ();
+ static __int128_t d = (unsigned long) &&a - (unsigned long) &&b;
+ static unsigned long e = (unsigned long) &&a - (unsigned long) &&b;
+ *p = &e;
+ return &d;
+}
+
+int
+main ()
+{
+ __int128_t *p;
+ unsigned long *q;
+ p = foo (&q);
+ if (*p != *q)
+ __builtin_abort ();
+ return 0;
+}
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix expansion of initializer extensions (PR c/80163)
2017-03-24 19:00 [PATCH] Fix expansion of initializer extensions (PR c/80163) Jakub Jelinek
@ 2017-03-27 15:52 ` Jeff Law
2017-03-31 6:37 ` Jakub Jelinek
0 siblings, 1 reply; 3+ messages in thread
From: Jeff Law @ 2017-03-27 15:52 UTC (permalink / raw)
To: Jakub Jelinek, Bernd Schmidt; +Cc: gcc-patches
On 03/24/2017 12:58 PM, Jakub Jelinek wrote:
> Hi!
>
> As can be seen on the following testcase, when expanding an extension
> in EXPAND_INITIALIZER context, we emit wrong extension operation
> (one depending on the signedness of the result type rather than
> on the signedness of the argument type, so e.g. extension of
> unsigned int to long long int is done using SIGN_EXTEND instead of
> ZERO_EXTEND, and extension of int to unsigned long long int using
> ZERO_EXTEND instead of SIGN_EXTEND.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
> ok for trunk?
>
> 2017-03-24 Jakub Jelinek <jakub@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?
treeop0 is the first source operand, isn't it?
Am I missing something here?
jeff
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix expansion of initializer extensions (PR c/80163)
2017-03-27 15:52 ` Jeff Law
@ 2017-03-31 6:37 ` Jakub Jelinek
0 siblings, 0 replies; 3+ messages in thread
From: Jakub Jelinek @ 2017-03-31 6:37 UTC (permalink / raw)
To: Jeff Law, Bernd Schmidt; +Cc: gcc-patches
Hi!
On Mon, Mar 27, 2017 at 09:51:27AM -0600, Jeff Law wrote:
> > 2017-03-24 Jakub Jelinek <jakub@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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-03-31 6:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-24 19:00 [PATCH] Fix expansion of initializer extensions (PR c/80163) Jakub Jelinek
2017-03-27 15:52 ` Jeff Law
2017-03-31 6:37 ` Jakub Jelinek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).