* [PATCH] sccvn: Fix up push_partial_def little-endian bitfield handling [PR97764]
@ 2020-11-10 9:11 Jakub Jelinek
2020-11-10 10:01 ` Richard Biener
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2020-11-10 9:11 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
Hi!
This patch fixes a thinko in the left-endian push_partial_def path.
As the testcase shows, we have 3 bitfields in the struct,
bitoff bitsize
0 3
3 28
31 1
the corresponding read is the byte at offset 3 (i.e. 24 bits)
and push_partial_def first handles the full store ({}) to all bits
and then is processing the store to the middle bitfield with value of -1.
Here are the interesting spots:
pd.offset -= offseti;
this adjusts the pd to { -21, 28 }, the (for little-endian lowest) 21
bits aren't interesting to us, we only care about the upper 7.
len = native_encode_expr (pd.rhs, this_buffer, bufsize,
MAX (0, -pd.offset) / BITS_PER_UNIT);
native_encode_expr has the offset parameter in bytes and we tell it
that we aren't interested in the first (lowest) two bytes of the number.
It encodes 0xff, 0xff with len == 2 then.
HOST_WIDE_INT size = pd.size;
if (pd.offset < 0)
size -= ROUND_DOWN (-pd.offset, BITS_PER_UNIT);
we get 28 - 16, i.e. 12 - the 16 is subtracting those 2 bytes that we
omitted in native_encode_expr.
size = MIN (size, (HOST_WIDE_INT) needed_len * BITS_PER_UNIT);
needed_len is how many bytes the read at most needs, and that is 1,
so we get size 8 and copy all 8 bits (i.e. a single byte plus nothing)
from the native_encode_expr filled this_buffer; this incorrectly sets
the byte to 0xff when we want 0x7f. The above line is correct for the
pd.offset >= 0 case when we don't skip anything, but for the pd.offset < 0
case we need to subtract also the remainder of the bits we aren't interested
in (the code shifts the bytes by that number of bits).
If it weren't for the big-endian path, we could as well do
if (pd.offset < 0)
size += pd.offset;
but the big-endian path needs it differently.
With the following patch, amnt is 3 and we subtract from 12 the (8 - 3)
bits and thus get the 7 which is the value we want.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/10?
2020-11-10 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/97764
* tree-ssa-sccvn.c (vn_walk_cb_data::push_partial_def): For
little-endian stores with negative pd.offset, subtract
BITS_PER_UNIT - amnt from size if amnt is non-zero.
* gcc.c-torture/execute/pr97764.c: New test.
--- gcc/tree-ssa-sccvn.c.jj 2020-11-06 20:31:34.298252809 +0100
+++ gcc/tree-ssa-sccvn.c 2020-11-09 14:45:00.454445644 +0100
@@ -2039,12 +2039,12 @@ vn_walk_cb_data::push_partial_def (pd_da
}
else
{
- size = MIN (size, (HOST_WIDE_INT) needed_len * BITS_PER_UNIT);
if (pd.offset >= 0)
{
/* LSB of this_buffer[0] byte should be at pd.offset bits
in buffer. */
unsigned int msk;
+ size = MIN (size, (HOST_WIDE_INT) needed_len * BITS_PER_UNIT);
amnt = pd.offset % BITS_PER_UNIT;
if (amnt)
shift_bytes_in_array_left (this_buffer, len + 1, amnt);
@@ -2074,6 +2074,9 @@ vn_walk_cb_data::push_partial_def (pd_da
{
amnt = (unsigned HOST_WIDE_INT) pd.offset % BITS_PER_UNIT;
if (amnt)
+ size -= BITS_PER_UNIT - amnt;
+ size = MIN (size, (HOST_WIDE_INT) needed_len * BITS_PER_UNIT);
+ if (amnt)
shift_bytes_in_array_left (this_buffer, len + 1, amnt);
}
memcpy (p, this_buffer + (amnt != 0), size / BITS_PER_UNIT);
--- gcc/testsuite/gcc.c-torture/execute/pr97764.c.jj 2020-11-09 14:36:20.974258322 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr97764.c 2020-11-09 14:36:56.108865088 +0100
@@ -0,0 +1,14 @@
+/* PR tree-optimization/97764 */
+/* { dg-require-effective-target int32plus } */
+
+struct S { int b : 3; int c : 28; int d : 1; };
+
+int
+main ()
+{
+ struct S e = {};
+ e.c = -1;
+ if (e.d)
+ __builtin_abort ();
+ return 0;
+}
Jakub
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] sccvn: Fix up push_partial_def little-endian bitfield handling [PR97764]
2020-11-10 9:11 [PATCH] sccvn: Fix up push_partial_def little-endian bitfield handling [PR97764] Jakub Jelinek
@ 2020-11-10 10:01 ` Richard Biener
0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2020-11-10 10:01 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Tue, 10 Nov 2020, Jakub Jelinek wrote:
> Hi!
>
> This patch fixes a thinko in the left-endian push_partial_def path.
> As the testcase shows, we have 3 bitfields in the struct,
> bitoff bitsize
> 0 3
> 3 28
> 31 1
> the corresponding read is the byte at offset 3 (i.e. 24 bits)
> and push_partial_def first handles the full store ({}) to all bits
> and then is processing the store to the middle bitfield with value of -1.
> Here are the interesting spots:
> pd.offset -= offseti;
> this adjusts the pd to { -21, 28 }, the (for little-endian lowest) 21
> bits aren't interesting to us, we only care about the upper 7.
> len = native_encode_expr (pd.rhs, this_buffer, bufsize,
> MAX (0, -pd.offset) / BITS_PER_UNIT);
> native_encode_expr has the offset parameter in bytes and we tell it
> that we aren't interested in the first (lowest) two bytes of the number.
> It encodes 0xff, 0xff with len == 2 then.
> HOST_WIDE_INT size = pd.size;
> if (pd.offset < 0)
> size -= ROUND_DOWN (-pd.offset, BITS_PER_UNIT);
> we get 28 - 16, i.e. 12 - the 16 is subtracting those 2 bytes that we
> omitted in native_encode_expr.
> size = MIN (size, (HOST_WIDE_INT) needed_len * BITS_PER_UNIT);
> needed_len is how many bytes the read at most needs, and that is 1,
> so we get size 8 and copy all 8 bits (i.e. a single byte plus nothing)
> from the native_encode_expr filled this_buffer; this incorrectly sets
> the byte to 0xff when we want 0x7f. The above line is correct for the
> pd.offset >= 0 case when we don't skip anything, but for the pd.offset < 0
> case we need to subtract also the remainder of the bits we aren't interested
> in (the code shifts the bytes by that number of bits).
> If it weren't for the big-endian path, we could as well do
> if (pd.offset < 0)
> size += pd.offset;
> but the big-endian path needs it differently.
> With the following patch, amnt is 3 and we subtract from 12 the (8 - 3)
> bits and thus get the 7 which is the value we want.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/10?
OK.
Thanks,
Richard.
> 2020-11-10 Jakub Jelinek <jakub@redhat.com>
>
> PR tree-optimization/97764
> * tree-ssa-sccvn.c (vn_walk_cb_data::push_partial_def): For
> little-endian stores with negative pd.offset, subtract
> BITS_PER_UNIT - amnt from size if amnt is non-zero.
>
> * gcc.c-torture/execute/pr97764.c: New test.
>
> --- gcc/tree-ssa-sccvn.c.jj 2020-11-06 20:31:34.298252809 +0100
> +++ gcc/tree-ssa-sccvn.c 2020-11-09 14:45:00.454445644 +0100
> @@ -2039,12 +2039,12 @@ vn_walk_cb_data::push_partial_def (pd_da
> }
> else
> {
> - size = MIN (size, (HOST_WIDE_INT) needed_len * BITS_PER_UNIT);
> if (pd.offset >= 0)
> {
> /* LSB of this_buffer[0] byte should be at pd.offset bits
> in buffer. */
> unsigned int msk;
> + size = MIN (size, (HOST_WIDE_INT) needed_len * BITS_PER_UNIT);
> amnt = pd.offset % BITS_PER_UNIT;
> if (amnt)
> shift_bytes_in_array_left (this_buffer, len + 1, amnt);
> @@ -2074,6 +2074,9 @@ vn_walk_cb_data::push_partial_def (pd_da
> {
> amnt = (unsigned HOST_WIDE_INT) pd.offset % BITS_PER_UNIT;
> if (amnt)
> + size -= BITS_PER_UNIT - amnt;
> + size = MIN (size, (HOST_WIDE_INT) needed_len * BITS_PER_UNIT);
> + if (amnt)
> shift_bytes_in_array_left (this_buffer, len + 1, amnt);
> }
> memcpy (p, this_buffer + (amnt != 0), size / BITS_PER_UNIT);
> --- gcc/testsuite/gcc.c-torture/execute/pr97764.c.jj 2020-11-09 14:36:20.974258322 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr97764.c 2020-11-09 14:36:56.108865088 +0100
> @@ -0,0 +1,14 @@
> +/* PR tree-optimization/97764 */
> +/* { dg-require-effective-target int32plus } */
> +
> +struct S { int b : 3; int c : 28; int d : 1; };
> +
> +int
> +main ()
> +{
> + struct S e = {};
> + e.c = -1;
> + if (e.d)
> + __builtin_abort ();
> + return 0;
> +}
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-11-10 10:01 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-10 9:11 [PATCH] sccvn: Fix up push_partial_def little-endian bitfield handling [PR97764] Jakub Jelinek
2020-11-10 10:01 ` 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).