* [PATCH] Fill bitregion_{start,end} in store_constructor (PR, tree-optimization/78428).
@ 2016-11-23 10:28 Martin Liška
2016-11-23 10:49 ` Richard Biener
0 siblings, 1 reply; 10+ messages in thread
From: Martin Liška @ 2016-11-23 10:28 UTC (permalink / raw)
To: GCC Patches; +Cc: Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 368 bytes --]
Following patch fixes situation where we do a store to a bitfield which
is at boundary of a record. This leads to usage of wider store, leading
to overwriting a following memory location.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
Apart from that, the reported test-case in PR works on x86_64-linux-gnu.
Ready to be installed?
Martin
[-- Attachment #2: 0001-Fill-bitregion_-start-end-in-store_constructor-PR-tr.patch --]
[-- Type: text/x-patch, Size: 4834 bytes --]
From 69961d69e551d9abe31eb7946d5211b99bbe1479 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 22 Nov 2016 10:15:42 +0100
Subject: [PATCH] Fill bitregion_{start,end} in store_constructor (PR
tree-optimization/78428).
gcc/testsuite/ChangeLog:
2016-11-22 Martin Liska <mliska@suse.cz>
Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/78428
* gcc.dg/tree-ssa/pr78428.c: New test.
gcc/ChangeLog:
2016-11-22 Martin Liska <mliska@suse.cz>
Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/78428
* expr.c (store_constructor_field): Add new arguments.
(store_constructor): Fill up store_constructor_field for
FIELD_DECL of a store_constructor_fielductor.
---
gcc/expr.c | 26 ++++++++++++++++++--------
gcc/testsuite/gcc.dg/tree-ssa/pr78428.c | 27 +++++++++++++++++++++++++++
2 files changed, 45 insertions(+), 8 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr78428.c
diff --git a/gcc/expr.c b/gcc/expr.c
index fe752fb..50bc7c5 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -80,7 +80,8 @@ static void clear_by_pieces (rtx, unsigned HOST_WIDE_INT, unsigned int);
static rtx_insn *compress_float_constant (rtx, rtx);
static rtx get_subtarget (rtx);
static void store_constructor_field (rtx, unsigned HOST_WIDE_INT,
- HOST_WIDE_INT, machine_mode,
+ HOST_WIDE_INT, unsigned HOST_WIDE_INT,
+ unsigned HOST_WIDE_INT, machine_mode,
tree, int, alias_set_type, bool);
static void store_constructor (tree, rtx, int, HOST_WIDE_INT, bool);
static rtx store_field (rtx, HOST_WIDE_INT, HOST_WIDE_INT,
@@ -6077,7 +6078,10 @@ all_zeros_p (const_tree exp)
static void
store_constructor_field (rtx target, unsigned HOST_WIDE_INT bitsize,
- HOST_WIDE_INT bitpos, machine_mode mode,
+ HOST_WIDE_INT bitpos,
+ unsigned HOST_WIDE_INT bitregion_start,
+ unsigned HOST_WIDE_INT bitregion_end,
+ machine_mode mode,
tree exp, int cleared,
alias_set_type alias_set, bool reverse)
{
@@ -6112,8 +6116,8 @@ store_constructor_field (rtx target, unsigned HOST_WIDE_INT bitsize,
reverse);
}
else
- store_field (target, bitsize, bitpos, 0, 0, mode, exp, alias_set, false,
- reverse);
+ store_field (target, bitsize, bitpos, bitregion_start, bitregion_end, mode,
+ exp, alias_set, false, reverse);
}
@@ -6148,6 +6152,8 @@ store_constructor (tree exp, rtx target, int cleared, HOST_WIDE_INT size,
{
tree type = TREE_TYPE (exp);
HOST_WIDE_INT exp_size = int_size_in_bytes (type);
+ HOST_WIDE_INT bitregion_end
+ = exp_size == -1 ? 0 : exp_size * BITS_PER_UNIT - 1;
switch (TREE_CODE (type))
{
@@ -6308,7 +6314,8 @@ store_constructor (tree exp, rtx target, int cleared, HOST_WIDE_INT size,
MEM_KEEP_ALIAS_SET_P (to_rtx) = 1;
}
- store_constructor_field (to_rtx, bitsize, bitpos, mode,
+ store_constructor_field (to_rtx, bitsize, bitpos,
+ 0, bitregion_end, mode,
value, cleared,
get_alias_set (TREE_TYPE (field)),
reverse);
@@ -6468,7 +6475,8 @@ store_constructor (tree exp, rtx target, int cleared, HOST_WIDE_INT size,
}
store_constructor_field
- (target, bitsize, bitpos, mode, value, cleared,
+ (target, bitsize, bitpos, 0, bitregion_end,
+ mode, value, cleared,
get_alias_set (elttype), reverse);
}
}
@@ -6571,7 +6579,8 @@ store_constructor (tree exp, rtx target, int cleared, HOST_WIDE_INT size,
target = copy_rtx (target);
MEM_KEEP_ALIAS_SET_P (target) = 1;
}
- store_constructor_field (target, bitsize, bitpos, mode, value,
+ store_constructor_field (target, bitsize, bitpos, 0,
+ bitregion_end, mode, value,
cleared, get_alias_set (elttype),
reverse);
}
@@ -6705,7 +6714,8 @@ store_constructor (tree exp, rtx target, int cleared, HOST_WIDE_INT size,
? TYPE_MODE (TREE_TYPE (value))
: eltmode;
bitpos = eltpos * elt_size;
- store_constructor_field (target, bitsize, bitpos, value_mode,
+ store_constructor_field (target, bitsize, bitpos, 0,
+ bitregion_end, value_mode,
value, cleared, alias, reverse);
}
}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr78428.c b/gcc/testsuite/gcc.dg/tree-ssa/pr78428.c
new file mode 100644
index 0000000..3a9b99c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr78428.c
@@ -0,0 +1,27 @@
+/* PR tree-optimization/78428. */
+/* { dg-options "-O2" } */
+/* { dg-do run } */
+
+struct S0
+{
+ int f2;
+ int f3:16;
+ int f4:18;
+} ;
+
+int a = 5;
+struct S0 b = { 3, 0, 0 };
+static struct S0 global[2] = { { 77, 0, 78 }, { 77, 0, 78 } };
+
+int main ()
+{
+ volatile struct S0 *j;
+ for (; a;)
+ {
+ __builtin_printf ("", b.f2);
+ j = &b;
+ *j = global[1];
+ a--;
+ }
+ return 0;
+}
--
2.10.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Fill bitregion_{start,end} in store_constructor (PR, tree-optimization/78428).
2016-11-23 10:28 [PATCH] Fill bitregion_{start,end} in store_constructor (PR, tree-optimization/78428) Martin Liška
@ 2016-11-23 10:49 ` Richard Biener
2016-12-06 17:52 ` Martin Liška
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Richard Biener @ 2016-11-23 10:49 UTC (permalink / raw)
To: Martin Liška, Eric Botcazou; +Cc: GCC Patches, Jakub Jelinek
On Wed, Nov 23, 2016 at 11:26 AM, Martin Liška <mliska@suse.cz> wrote:
> Following patch fixes situation where we do a store to a bitfield which
> is at boundary of a record. This leads to usage of wider store, leading
> to overwriting a following memory location.
>
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> Apart from that, the reported test-case in PR works on x86_64-linux-gnu.
>
> Ready to be installed?
+ HOST_WIDE_INT bitregion_end
+ = exp_size == -1 ? 0 : exp_size * BITS_PER_UNIT - 1;
I don't think looking at the CONSTRUCTOR to determine bitregion_end is
a good idea.
The function gets 'size' as argument which is documented as "number of
bytes we are
allowed to modify" - so better use that.
@@ -6308,7 +6314,8 @@ store_constructor (tree exp, rtx target, int
cleared, HOST_WIDE_INT size,
MEM_KEEP_ALIAS_SET_P (to_rtx) = 1;
}
- store_constructor_field (to_rtx, bitsize, bitpos, mode,
+ store_constructor_field (to_rtx, bitsize, bitpos,
+ 0, bitregion_end, mode,
value, cleared,
get_alias_set (TREE_TYPE (field)),
reverse);
this stores to to_rtx which may be offsetted from target this means in this case
bitregion_end is not conservative enough - you'd need to resort to the
field width
in that case I guess (and for variable field size not specify any end
-- I suppose
the 'size' store_constructor gets might also be "unknown"?). But maybe all
the non-constant offset / size cases are "dead code" now that we are in GIMPLE?
Note they likely can only appear from Ada code anyway -- CCing Eric.
I suppose a "safe" thing to do would be to give up on the first
variable offset/size
and re-set bitregion_end to zero for this and all following fields.
The other cases look fine to me.
Thanks,
Richard.
> Martin
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Fill bitregion_{start,end} in store_constructor (PR, tree-optimization/78428).
2016-11-23 10:49 ` Richard Biener
@ 2016-12-06 17:52 ` Martin Liška
2016-12-06 18:02 ` Eric Botcazou
2016-12-09 10:20 ` Martin Liška
2 siblings, 0 replies; 10+ messages in thread
From: Martin Liška @ 2016-12-06 17:52 UTC (permalink / raw)
To: Richard Biener, Eric Botcazou; +Cc: GCC Patches, Jakub Jelinek
Pinging Eric.
On 11/23/2016 11:49 AM, Richard Biener wrote:
> On Wed, Nov 23, 2016 at 11:26 AM, Martin Liška <mliska@suse.cz> wrote:
>> Following patch fixes situation where we do a store to a bitfield which
>> is at boundary of a record. This leads to usage of wider store, leading
>> to overwriting a following memory location.
>>
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>> Apart from that, the reported test-case in PR works on x86_64-linux-gnu.
>>
>> Ready to be installed?
>
> + HOST_WIDE_INT bitregion_end
> + = exp_size == -1 ? 0 : exp_size * BITS_PER_UNIT - 1;
>
> I don't think looking at the CONSTRUCTOR to determine bitregion_end is
> a good idea.
> The function gets 'size' as argument which is documented as "number of
> bytes we are
> allowed to modify" - so better use that.
>
> @@ -6308,7 +6314,8 @@ store_constructor (tree exp, rtx target, int
> cleared, HOST_WIDE_INT size,
> MEM_KEEP_ALIAS_SET_P (to_rtx) = 1;
> }
>
> - store_constructor_field (to_rtx, bitsize, bitpos, mode,
> + store_constructor_field (to_rtx, bitsize, bitpos,
> + 0, bitregion_end, mode,
> value, cleared,
> get_alias_set (TREE_TYPE (field)),
> reverse);
>
> this stores to to_rtx which may be offsetted from target this means in this case
> bitregion_end is not conservative enough - you'd need to resort to the
> field width
> in that case I guess (and for variable field size not specify any end
> -- I suppose
> the 'size' store_constructor gets might also be "unknown"?). But maybe all
> the non-constant offset / size cases are "dead code" now that we are in GIMPLE?
> Note they likely can only appear from Ada code anyway -- CCing Eric.
>
> I suppose a "safe" thing to do would be to give up on the first
> variable offset/size
> and re-set bitregion_end to zero for this and all following fields.
>
> The other cases look fine to me.
>
> Thanks,
> Richard.
>
>> Martin
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Fill bitregion_{start,end} in store_constructor (PR, tree-optimization/78428).
2016-11-23 10:49 ` Richard Biener
2016-12-06 17:52 ` Martin Liška
@ 2016-12-06 18:02 ` Eric Botcazou
2016-12-09 10:20 ` Martin Liška
2 siblings, 0 replies; 10+ messages in thread
From: Eric Botcazou @ 2016-12-06 18:02 UTC (permalink / raw)
To: Richard Biener, Martin Liška; +Cc: gcc-patches, Jakub Jelinek
> But maybe all the non-constant offset / size cases are "dead code" now that
> we are in GIMPLE? Note they likely can only appear from Ada code anyway
Possibly, but to be sure put a call to gcc_unreachable where appropriate and
run the Ada testsuite; if it is clean, then send me the patch and I'll run it
on a more thorough testsuite.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Fill bitregion_{start,end} in store_constructor (PR, tree-optimization/78428).
2016-11-23 10:49 ` Richard Biener
2016-12-06 17:52 ` Martin Liška
2016-12-06 18:02 ` Eric Botcazou
@ 2016-12-09 10:20 ` Martin Liška
2016-12-12 11:10 ` Eric Botcazou
2 siblings, 1 reply; 10+ messages in thread
From: Martin Liška @ 2016-12-09 10:20 UTC (permalink / raw)
To: Richard Biener, Eric Botcazou; +Cc: GCC Patches, Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 2459 bytes --]
On 11/23/2016 11:49 AM, Richard Biener wrote:
> On Wed, Nov 23, 2016 at 11:26 AM, Martin Liška <mliska@suse.cz> wrote:
>> Following patch fixes situation where we do a store to a bitfield which
>> is at boundary of a record. This leads to usage of wider store, leading
>> to overwriting a following memory location.
>>
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>> Apart from that, the reported test-case in PR works on x86_64-linux-gnu.
>>
>> Ready to be installed?
>
> + HOST_WIDE_INT bitregion_end
> + = exp_size == -1 ? 0 : exp_size * BITS_PER_UNIT - 1;
>
> I don't think looking at the CONSTRUCTOR to determine bitregion_end is
> a good idea.
> The function gets 'size' as argument which is documented as "number of
> bytes we are
> allowed to modify" - so better use that.
>
> @@ -6308,7 +6314,8 @@ store_constructor (tree exp, rtx target, int
> cleared, HOST_WIDE_INT size,
> MEM_KEEP_ALIAS_SET_P (to_rtx) = 1;
> }
>
> - store_constructor_field (to_rtx, bitsize, bitpos, mode,
> + store_constructor_field (to_rtx, bitsize, bitpos,
> + 0, bitregion_end, mode,
> value, cleared,
> get_alias_set (TREE_TYPE (field)),
> reverse);
>
> this stores to to_rtx which may be offsetted from target this means in this case
> bitregion_end is not conservative enough - you'd need to resort to the
> field width
> in that case I guess (and for variable field size not specify any end
> -- I suppose
> the 'size' store_constructor gets might also be "unknown"?). But maybe all
> the non-constant offset / size cases are "dead code" now that we are in GIMPLE?
> Note they likely can only appear from Ada code anyway -- CCing Eric.
>
> I suppose a "safe" thing to do would be to give up on the first
> variable offset/size
> and re-set bitregion_end to zero for this and all following fields.
Ok. I'm sending a patch that put gcc_unreachable to places where either size
or (and) offset is a non-constant. This survives regression tests (including ada)
on x86_64-linux-gnu. Apart from that normal bootstrap + regression tests works fine
on ppc64le-redhat-linux.
I'm not sure whether there can be a code that has variable size and offset?
Martin
>
> The other cases look fine to me.
>
> Thanks,
> Richard.
>
>> Martin
[-- Attachment #2: 0001-Fill-bitregion_-start-end-in-store_constructor-PR-tr.patch --]
[-- Type: text/x-patch, Size: 6060 bytes --]
From c8d3e7c33c22bfdaf9e285e9e747701b8646cdf2 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 22 Nov 2016 10:15:42 +0100
Subject: [PATCH] Fill bitregion_{start,end} in store_constructor (PR
tree-optimization/78428).
gcc/ChangeLog:
2016-12-08 Martin Liska <mliska@suse.cz>
* expr.c (store_constructor_field): Add new arguments to the
function.
(store_constructor): Set up bitregion_end and add
gcc_unreachable to fields that have either non-constant size
or (and) offset.
gcc/testsuite/ChangeLog:
2016-12-08 Martin Liska <mliska@suse.cz>
* gcc.dg/tree-ssa/pr78428.c: New test.
---
gcc/expr.c | 52 +++++++++++++--------------------
gcc/testsuite/gcc.dg/tree-ssa/pr78428.c | 27 +++++++++++++++++
2 files changed, 47 insertions(+), 32 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr78428.c
diff --git a/gcc/expr.c b/gcc/expr.c
index fe752fb..88da8dd 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -80,7 +80,8 @@ static void clear_by_pieces (rtx, unsigned HOST_WIDE_INT, unsigned int);
static rtx_insn *compress_float_constant (rtx, rtx);
static rtx get_subtarget (rtx);
static void store_constructor_field (rtx, unsigned HOST_WIDE_INT,
- HOST_WIDE_INT, machine_mode,
+ HOST_WIDE_INT, unsigned HOST_WIDE_INT,
+ unsigned HOST_WIDE_INT, machine_mode,
tree, int, alias_set_type, bool);
static void store_constructor (tree, rtx, int, HOST_WIDE_INT, bool);
static rtx store_field (rtx, HOST_WIDE_INT, HOST_WIDE_INT,
@@ -6077,7 +6078,10 @@ all_zeros_p (const_tree exp)
static void
store_constructor_field (rtx target, unsigned HOST_WIDE_INT bitsize,
- HOST_WIDE_INT bitpos, machine_mode mode,
+ HOST_WIDE_INT bitpos,
+ unsigned HOST_WIDE_INT bitregion_start,
+ unsigned HOST_WIDE_INT bitregion_end,
+ machine_mode mode,
tree exp, int cleared,
alias_set_type alias_set, bool reverse)
{
@@ -6112,8 +6116,8 @@ store_constructor_field (rtx target, unsigned HOST_WIDE_INT bitsize,
reverse);
}
else
- store_field (target, bitsize, bitpos, 0, 0, mode, exp, alias_set, false,
- reverse);
+ store_field (target, bitsize, bitpos, bitregion_start, bitregion_end, mode,
+ exp, alias_set, false, reverse);
}
@@ -6148,6 +6152,7 @@ store_constructor (tree exp, rtx target, int cleared, HOST_WIDE_INT size,
{
tree type = TREE_TYPE (exp);
HOST_WIDE_INT exp_size = int_size_in_bytes (type);
+ HOST_WIDE_INT bitregion_end = size > 0 ? size * BITS_PER_UNIT - 1 : 0;
switch (TREE_CODE (type))
{
@@ -6225,7 +6230,7 @@ store_constructor (tree exp, rtx target, int cleared, HOST_WIDE_INT size,
if (tree_fits_uhwi_p (DECL_SIZE (field)))
bitsize = tree_to_uhwi (DECL_SIZE (field));
else
- bitsize = -1;
+ gcc_unreachable ();
mode = DECL_MODE (field);
if (DECL_BIT_FIELD (field))
@@ -6236,31 +6241,10 @@ store_constructor (tree exp, rtx target, int cleared, HOST_WIDE_INT size,
&& tree_fits_shwi_p (bit_position (field)))
{
bitpos = int_bit_position (field);
- offset = 0;
+ offset = NULL_TREE;
}
else
- bitpos = tree_to_shwi (DECL_FIELD_BIT_OFFSET (field));
-
- if (offset)
- {
- machine_mode address_mode;
- rtx offset_rtx;
-
- offset
- = SUBSTITUTE_PLACEHOLDER_IN_EXPR (offset,
- make_tree (TREE_TYPE (exp),
- target));
-
- offset_rtx = expand_normal (offset);
- gcc_assert (MEM_P (to_rtx));
-
- address_mode = get_address_mode (to_rtx);
- if (GET_MODE (offset_rtx) != address_mode)
- offset_rtx = convert_to_mode (address_mode, offset_rtx, 0);
-
- to_rtx = offset_address (to_rtx, offset_rtx,
- highest_pow2_factor (offset));
- }
+ gcc_unreachable ();
/* If this initializes a field that is smaller than a
word, at the start of a word, try to widen it to a full
@@ -6308,7 +6292,8 @@ store_constructor (tree exp, rtx target, int cleared, HOST_WIDE_INT size,
MEM_KEEP_ALIAS_SET_P (to_rtx) = 1;
}
- store_constructor_field (to_rtx, bitsize, bitpos, mode,
+ store_constructor_field (to_rtx, bitsize, bitpos,
+ 0, bitregion_end, mode,
value, cleared,
get_alias_set (TREE_TYPE (field)),
reverse);
@@ -6468,7 +6453,8 @@ store_constructor (tree exp, rtx target, int cleared, HOST_WIDE_INT size,
}
store_constructor_field
- (target, bitsize, bitpos, mode, value, cleared,
+ (target, bitsize, bitpos, 0, bitregion_end,
+ mode, value, cleared,
get_alias_set (elttype), reverse);
}
}
@@ -6571,7 +6557,8 @@ store_constructor (tree exp, rtx target, int cleared, HOST_WIDE_INT size,
target = copy_rtx (target);
MEM_KEEP_ALIAS_SET_P (target) = 1;
}
- store_constructor_field (target, bitsize, bitpos, mode, value,
+ store_constructor_field (target, bitsize, bitpos, 0,
+ bitregion_end, mode, value,
cleared, get_alias_set (elttype),
reverse);
}
@@ -6705,7 +6692,8 @@ store_constructor (tree exp, rtx target, int cleared, HOST_WIDE_INT size,
? TYPE_MODE (TREE_TYPE (value))
: eltmode;
bitpos = eltpos * elt_size;
- store_constructor_field (target, bitsize, bitpos, value_mode,
+ store_constructor_field (target, bitsize, bitpos, 0,
+ bitregion_end, value_mode,
value, cleared, alias, reverse);
}
}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr78428.c b/gcc/testsuite/gcc.dg/tree-ssa/pr78428.c
new file mode 100644
index 0000000..3a9b99c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr78428.c
@@ -0,0 +1,27 @@
+/* PR tree-optimization/78428. */
+/* { dg-options "-O2" } */
+/* { dg-do run } */
+
+struct S0
+{
+ int f2;
+ int f3:16;
+ int f4:18;
+} ;
+
+int a = 5;
+struct S0 b = { 3, 0, 0 };
+static struct S0 global[2] = { { 77, 0, 78 }, { 77, 0, 78 } };
+
+int main ()
+{
+ volatile struct S0 *j;
+ for (; a;)
+ {
+ __builtin_printf ("", b.f2);
+ j = &b;
+ *j = global[1];
+ a--;
+ }
+ return 0;
+}
--
2.10.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Fill bitregion_{start,end} in store_constructor (PR, tree-optimization/78428).
2016-12-09 10:20 ` Martin Liška
@ 2016-12-12 11:10 ` Eric Botcazou
2016-12-13 9:05 ` Martin Liška
0 siblings, 1 reply; 10+ messages in thread
From: Eric Botcazou @ 2016-12-12 11:10 UTC (permalink / raw)
To: Martin Liška; +Cc: gcc-patches, Richard Biener, Jakub Jelinek
> Ok. I'm sending a patch that put gcc_unreachable to places where either size
> or (and) offset is a non-constant. This survives regression tests
> (including ada) on x86_64-linux-gnu. Apart from that normal bootstrap +
> regression tests works fine on ppc64le-redhat-linux.
I didn't manage to break it so it is OK by me.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Fill bitregion_{start,end} in store_constructor (PR, tree-optimization/78428).
2016-12-12 11:10 ` Eric Botcazou
@ 2016-12-13 9:05 ` Martin Liška
2016-12-13 14:36 ` Richard Biener
0 siblings, 1 reply; 10+ messages in thread
From: Martin Liška @ 2016-12-13 9:05 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, Richard Biener, Jakub Jelinek
On 12/12/2016 12:10 PM, Eric Botcazou wrote:
>> Ok. I'm sending a patch that put gcc_unreachable to places where either size
>> or (and) offset is a non-constant. This survives regression tests
>> (including ada) on x86_64-linux-gnu. Apart from that normal bootstrap +
>> regression tests works fine on ppc64le-redhat-linux.
>
> I didn't manage to break it so it is OK by me.
>
Out of curiosity, I blame the commit which added the
if (offset)
{
machine_mode address_mode;
rtx offset_rtx;
offset
= SUBSTITUTE_PLACEHOLDER_IN_EXPR (offset,
make_tree (TREE_TYPE (exp),
target));
and it comes to commit done in 1993:
commit c869557a9ccc1bd3e5474b144bcb84065db23549
Author: kenner <kenner@138bc75d-0d04-0410-961f-82ee72b054a4>
Date: Mon Oct 4 01:48:03 1993 +0000
(store_expr): Use expr_size value, not size_int.
(store_constructor): Handle case of variable position and allow it to contain
a PLACEHOLDER_EXPR.
(get_inner_reference): Make a WITH_RECORD_EXPR if required.
(expand_expr, case PLACEHOLDER_EXPR, WITH_RECORD_EXPR): New cases.
(expand_expr, case ARRAY_REF): Make WITH_RECORD_EXPR expressions when needed.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@5584 138bc75d-0d04-0410-961f-82ee72b054a4
May I install the patch?
Martin
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Fill bitregion_{start,end} in store_constructor (PR, tree-optimization/78428).
2016-12-13 9:05 ` Martin Liška
@ 2016-12-13 14:36 ` Richard Biener
2016-12-15 11:04 ` Martin Liška
0 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2016-12-13 14:36 UTC (permalink / raw)
To: Martin Liška; +Cc: Eric Botcazou, GCC Patches, Jakub Jelinek
On Tue, Dec 13, 2016 at 10:05 AM, Martin Liška <mliska@suse.cz> wrote:
> On 12/12/2016 12:10 PM, Eric Botcazou wrote:
>>> Ok. I'm sending a patch that put gcc_unreachable to places where either size
>>> or (and) offset is a non-constant. This survives regression tests
>>> (including ada) on x86_64-linux-gnu. Apart from that normal bootstrap +
>>> regression tests works fine on ppc64le-redhat-linux.
>>
>> I didn't manage to break it so it is OK by me.
>>
>
> Out of curiosity, I blame the commit which added the
>
> if (offset)
> {
> machine_mode address_mode;
> rtx offset_rtx;
>
> offset
> = SUBSTITUTE_PLACEHOLDER_IN_EXPR (offset,
> make_tree (TREE_TYPE (exp),
> target));
>
> and it comes to commit done in 1993:
>
> commit c869557a9ccc1bd3e5474b144bcb84065db23549
> Author: kenner <kenner@138bc75d-0d04-0410-961f-82ee72b054a4>
> Date: Mon Oct 4 01:48:03 1993 +0000
>
> (store_expr): Use expr_size value, not size_int.
> (store_constructor): Handle case of variable position and allow it to contain
> a PLACEHOLDER_EXPR.
> (get_inner_reference): Make a WITH_RECORD_EXPR if required.
> (expand_expr, case PLACEHOLDER_EXPR, WITH_RECORD_EXPR): New cases.
> (expand_expr, case ARRAY_REF): Make WITH_RECORD_EXPR expressions when needed.
Yeah, I expect that we got rid of the cases at the time we introduced
GIMPLE (and gimplification).
>
> git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@5584 138bc75d-0d04-0410-961f-82ee72b054a4
>
> May I install the patch?
OK from my POV.
Thanks,
Richard.
> Martin
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Fill bitregion_{start,end} in store_constructor (PR, tree-optimization/78428).
2016-12-13 14:36 ` Richard Biener
@ 2016-12-15 11:04 ` Martin Liška
2016-12-16 13:07 ` Richard Biener
0 siblings, 1 reply; 10+ messages in thread
From: Martin Liška @ 2016-12-15 11:04 UTC (permalink / raw)
To: Richard Biener; +Cc: Eric Botcazou, GCC Patches, Jakub Jelinek
On 12/13/2016 03:35 PM, Richard Biener wrote:
> OK from my POV.
>
> Thanks,
> Richard.
Hi.
I prepared backport (and tested) for GCC 5 and 6 branches,
may I install the patch after 6.3 will be released?
Thanks,
Martin
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Fill bitregion_{start,end} in store_constructor (PR, tree-optimization/78428).
2016-12-15 11:04 ` Martin Liška
@ 2016-12-16 13:07 ` Richard Biener
0 siblings, 0 replies; 10+ messages in thread
From: Richard Biener @ 2016-12-16 13:07 UTC (permalink / raw)
To: Martin Liška; +Cc: Eric Botcazou, GCC Patches, Jakub Jelinek
On Thu, Dec 15, 2016 at 12:00 PM, Martin Liška <mliska@suse.cz> wrote:
> On 12/13/2016 03:35 PM, Richard Biener wrote:
>> OK from my POV.
>>
>> Thanks,
>> Richard.
>
> Hi.
>
> I prepared backport (and tested) for GCC 5 and 6 branches,
> may I install the patch after 6.3 will be released?
Ok for early next year.
Thanks,
Richard.
> Thanks,
> Martin
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-12-16 13:02 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-23 10:28 [PATCH] Fill bitregion_{start,end} in store_constructor (PR, tree-optimization/78428) Martin Liška
2016-11-23 10:49 ` Richard Biener
2016-12-06 17:52 ` Martin Liška
2016-12-06 18:02 ` Eric Botcazou
2016-12-09 10:20 ` Martin Liška
2016-12-12 11:10 ` Eric Botcazou
2016-12-13 9:05 ` Martin Liška
2016-12-13 14:36 ` Richard Biener
2016-12-15 11:04 ` Martin Liška
2016-12-16 13:07 ` Richard Biener
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).