Re: [PATCH] expansion: Sign or zero extend on MEM_REF stores into SUBREG with SUBREG_PROMOTED_VAR_P [PR98190]

[Jakub Jelinek via Gcc-patches]

On Thu, Dec 10, 2020 at 11:15:30AM +0100, Eric Botcazou wrote:
> Are you sure that the optimization is worth the hassle (and maybe the risk, 
> i.e. can't store_field clobber the entire field)?

I'm not sure, after all, only the testcases I've mentioned were affected
in the whole bootstrap/regtest.
And, if there is no insv, store_fixed_* might indeed expand it as
(to_rtx & mask) | value
which isn't 100% clear what it do to the bits beyond the bit-field.
It is true that CONST_INTs are sign-extended, so maybe it wouldn't clear
them for sign promoted to_rtx, but who knows.
> 
> Any particular reason not to use the canonical idiom at the end, i.e. just
> 
>         convert_move (SUBREG_REG (to_rtx), to_rtx1,
>                    SUBREG_PROMOTED_SIGN (to_rtx));

Ah, wasn't aware of that.

So like this instead (if it passes another bootstrap/regtest)?

2020-12-10  Jakub Jelinek  <ja...@redhat.com>

        PR middle-end/98190
        * expr.c (expand_assignment): If to_rtx is a promoted SUBREG,
        ensure sign or zero extension either through use of store_expr
        or by extending manually.

        * gcc.dg/pr98190.c: New test.

--- gcc/expr.c.jj       2020-12-09 23:50:41.385776978 +0100
+++ gcc/expr.c  2020-12-10 11:38:07.853189206 +0100
@@ -5451,6 +5451,30 @@ expand_assignment (tree to, tree from, b
                                               mode1, to_rtx, to, from,
                                               reversep))
            result = NULL;
+         else if (SUBREG_P (to_rtx)
+                  && SUBREG_PROMOTED_VAR_P (to_rtx))
+           {
+             /* If to_rtx is a promoted subreg, we need to zero or sign
+                extend the value afterwards.  */
+             if (TREE_CODE (to) == MEM_REF
+                 && !REF_REVERSE_STORAGE_ORDER (to)
+                 && known_eq (bitpos, 0)
+                 && known_eq (bitsize, GET_MODE_BITSIZE (GET_MODE (to_rtx))))
+               result = store_expr (from, to_rtx, 0, nontemporal, false);
+             else
+               {
+                 rtx to_rtx1
+                   = lowpart_subreg (subreg_unpromoted_mode (to_rtx),
+                                     SUBREG_REG (to_rtx),
+                                     subreg_promoted_mode (to_rtx));
+                 result = store_field (to_rtx1, bitsize, bitpos,
+                                       bitregion_start, bitregion_end,
+                                       mode1, from, get_alias_set (to),
+                                       nontemporal, reversep);
+                 convert_move (SUBREG_REG (to_rtx), to_rtx1,
+                               SUBREG_PROMOTED_SIGN (to_rtx));
+               }
+           }
          else
            result = store_field (to_rtx, bitsize, bitpos,
                                  bitregion_start, bitregion_end,
--- gcc/testsuite/gcc.dg/pr98190.c.jj   2020-12-10 09:08:54.838216477 +0100
+++ gcc/testsuite/gcc.dg/pr98190.c      2020-12-10 09:08:54.838216477 +0100
@@ -0,0 +1,33 @@
+/* PR middle-end/98190 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+static int __attribute__((noipa))
+foo (const char *p, const char *q, const int len)
+{
+  for (int i = 0; i < len; p++, q++, i++)
+    {
+      int equal;
+      _Bool x, y;
+      __builtin_memcpy ((char *) &x, p, sizeof x);
+      __builtin_memcpy ((char *) &y, q, sizeof y);
+      equal = (x == y);
+      if (equal <= 0)
+       return equal;
+    }
+  return 1;
+}
+
+int
+main ()
+{
+  const _Bool buf[4] = { 1, 0, 0, 0 };
+#ifdef __aarch64__
+  register long x4 asm ("x4") = 0xdeadbeefULL;
+  register long x5 asm ("x5") = 0xcafebabeULL;
+  asm volatile (""::"r" (x4), "r" (x5));
+#endif
+  if (foo ((char *) &buf[0], (char *) &buf[0], 1) != 1)
+    __builtin_abort ();
+  return 0;
+}


        Jakub