[PATCH][expr.c] PR middle-end/71700: zero-extend sub-word value when widening constructor element

[Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>]

[-- Attachment #1: Type: text/plain, Size: 2205 bytes --]

Hi all,

In this arm wrong-code PR the struct assignment goes wrong when expanding constructor elements to a register destination
when the constructor elements are signed bitfields less than a word wide.
In this testcase we're intialising a struct with a 16-bit signed bitfield to -1 followed by a 1-bit bitfield to 0.
Before it starts storing the elements it zeroes out the register.
  The code in store_constructor extends the first field to word size because it appears at the beginning of a word.
It sign-extends the -1 to word size. However, when it later tries to store the 0 to bitposition 16 it has some logic
to avoid redundant zeroing since the destination was originally cleared, so it doesn't emit the zero store.
But the previous sign-extended -1 took up the whole word, so the position of the second bitfield contains a set bit.

This patch fixes the problem by zeroing out the bits of the widened field that did not appear in the original value,
so that we can safely avoid storing the second zero in the constructor.

With this patch the testcase passes at all optimisation levels (it only passed at -O0 before).

The initialisation of the struct to {-1, 0} now emits the RTL:
(insn 5 2 6 2 (set (reg/v:SI 112 [ e ])
         (const_int 0 [0]))
      (nil))
(insn 6 5 7 2 (set (reg/v:SI 112 [ e ])
         (const_int 65535 [0xffff]))
      (nil))

whereas before it generated:
(insn 5 4 6 (set (reg/v:SI 112 [ e ])
         (const_int 0 [0]))
      (nil))

(insn 6 5 7 (set (reg/v:SI 112 [ e ])
         (const_int -1 [0xffffffffffffffff]))
      (nil))


Bootstrapped and tested on arm, aarch64, x86_64 though the codepath is gated on WORD_REGISTER_OPERATIONS I didn't
expect any effect on aarch64 and x86_64 anyway.

Ok for trunk?

This bug appears on all versions from 4.9 onwards so I'll be testing it on the branches as well.

Thanks,
Kyrill

2016-07-01  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR middle-end/71700
     * expr.c (store_constructor): Mask sign-extended bits when widening
     sub-word constructor element at the start of a word.

2016-07-01  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR middle-end/71700
     * gcc.c-torture/execute/pr71700.c: New test.

[-- Attachment #2: expr-widen.patch --]
[-- Type: text/x-patch, Size: 1319 bytes --]

diff --git a/gcc/expr.c b/gcc/expr.c
index 9733401e09052457678a6a8817fe16f8a737927e..abb4416e41b60ab69f6f9d844e3a40853b0d1cde 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -6281,6 +6281,13 @@ store_constructor (tree exp, rtx target, int cleared, HOST_WIDE_INT size,
 		    type = lang_hooks.types.type_for_mode
 		      (word_mode, TYPE_UNSIGNED (type));
 		    value = fold_convert (type, value);
+		    /* Make sure the bits beyond the original bitsize are zero
+		       so that we can correctly avoid extra zeroing stores in
+		       later constructor elements.  */
+		    tree bitsize_mask
+		      = wide_int_to_tree (type, wi::mask (bitsize, false,
+							   BITS_PER_WORD));
+		    value = fold_build2 (BIT_AND_EXPR, type, value, bitsize_mask);
 		  }
 
 		if (BYTES_BIG_ENDIAN)
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr71700.c b/gcc/testsuite/gcc.c-torture/execute/pr71700.c
new file mode 100644
index 0000000000000000000000000000000000000000..80afd3809c3abd24135fb36b757ace9f41ba3112
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr71700.c
@@ -0,0 +1,19 @@
+struct S
+{
+  signed f0 : 16;
+  unsigned f1 : 1;
+};
+
+int b;
+static struct S c[] = {{-1, 0}, {-1, 0}};
+struct S d;
+
+int
+main ()
+{
+  struct S e = c[0];
+  d = e;
+  if (d.f1 != 0)
+    __builtin_abort ();
+  return 0;
+}