PR middle-end/118608: Fix lack of sign extension on MIPS64 with -Os

Here is a refreshed version of my patch from February 2025, for resolving
PR middle-end/118608, a wrong code on valid regression where the middle-end
is failing to keep values suitably sign-extended (MIPS64 is a rare
targetm.mode_rep_extended target, as well as being BYTES_BIG_ENDIAN).

This fix requires three independent tweaks, one in each source file.
The first tweak is that the logic in my triggering patch for determining
whether store_field updates the most significant bit needs to be updated
to handle BYTES_BIG_ENDIAN.  Of the two insertions in the bugzilla test
case, we were generating the sign extension after the wrong one.
The second tweak was that this explicit sign-extension was then being
eliminated during combine by simplify-rtx that believed the explicit
TRUNCATE wasn't required.  This patch updates truncated_to_mode to
understand that on mode_rep_extended targets, TRUNCATE is used instead
of SUBREG because it isn't a no-op.  Finally, the third tweak is
that the MIPS backend requires a small change to recognize (and split)
*extenddi_truncatesi when TARGET_64BIT and !ISA_HAS_EXTS.

On mips64-elf with -mabi=64 the following are now generated for prepareNeedle:

-O2
        sll     $5,$5,16
        jr      $31
        or      $2,$5,$4

-Os
        dsll    $5,$5,16
        or      $2,$4,$5
        dsll    $2,$2,32
        jr      $31
        dsra    $2,$2,32

-O2 -march=octeon2
        move    $2,$0
        ins     $2,$5,16,16
        jr      $31
        ins     $2,$4,0,16

-Os -march=octeon2
        move    $2,$0
        dins    $2,$4,0,16
        dins    $2,$5,16,16
        jr      $31
        sll     $2,$2,0

Many thanks to Mateusz Marciniec and Jeff Law for additional testing.

gcc/ChangeLog
	PR middle-end/118608
	* expr.cc (store_field_updates_msb_p): New helper function that
	now also handles BYTES_BIG_ENDIAN targets.
	(expand_assignment): Use the above function when deciding to emit
	a required sign/zero extension.
	* rtlanal.cc (truncated_to_mode): Call targetm.mode_rep_extended
	to check whether an explicit TRUNCATE is required (i.e. performs
	an extension) on this target.
	* config/mips/mips.md (*extenddi_truncate<mode>): Handle all
	SUBDI modes, not just SHORT modes.

gcc/testsuite/ChangeLog
	PR middle-end/118608
	* gcc.target/mips/pr118608-1.c: New test case.
	* gcc.target/mips/pr118608-2.c: Likewise.
	* gcc.target/mips/pr118608-3.c: Likewise.
	* gcc.target/mips/pr118608-4.c: Likewise.

diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
index 85e7d67901f761193fe4ae73f154f598c0264487..6a1b63b00d2e41f595a6f0f07edb8dc4f1989fe0 100644 (file)
--- a/gcc/config/mips/mips.md
+++ b/gcc/config/mips/mips.md
@@ -3952,7 +3952,7 @@
 (define_insn_and_split "*extenddi_truncate<mode>"
   [(set (match_operand:DI 0 "register_operand" "=d")
        (sign_extend:DI
-           (truncate:SHORT (match_operand:DI 1 "register_operand" "d"))))]
+           (truncate:SUBDI (match_operand:DI 1 "register_operand" "d"))))]
   "TARGET_64BIT && !TARGET_MIPS16 && !ISA_HAS_EXTS"
   "#"
   "&& reload_completed"

diff --git a/gcc/expr.cc b/gcc/expr.cc
index c279341ffcbc75bbc6090429e010b9b701aa4c7c..4d1a6f3dd1ce794f574b6342d1a1a041ffa010f3 100644 (file)
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -5969,6 +5969,18 @@ mem_ref_refers_to_non_mem_p (tree ref)
   return non_mem_decl_p (base);
 }
 
+/* Helper function of expand_assignment.  Check if storing field of
+   size BITSIZE at position BITPOS overlaps with the most significant
+   bit of TO_RTX, known to be SUBREG_PROMOTED_VAR_P.
+   Updating this field requires an explicit extension.  */
+static bool
+store_field_updates_msb_p (poly_int64 bitpos, poly_int64 bitsize, rtx to_rtx)
+{
+  poly_int64 to_size = GET_MODE_SIZE (GET_MODE (to_rtx));
+  poly_int64 bitnum = BYTES_BIG_ENDIAN ? to_size - bitsize - bitpos : bitpos;
+  return maybe_eq (bitnum + bitsize, to_size);
+}
+
 /* Expand an assignment that stores the value of FROM into TO.  If NONTEMPORAL
    is true, try generating a nontemporal store.  */
 
@@ -6315,8 +6327,7 @@ expand_assignment (tree to, tree from, bool nontemporal)
                  && known_eq (bitsize, GET_MODE_BITSIZE (GET_MODE (to_rtx))))
                result = store_expr (from, to_rtx, 0, nontemporal, false);
              /* Check if the field overlaps the MSB, requiring extension.  */
-             else if (maybe_eq (bitpos + bitsize,
-                                GET_MODE_BITSIZE (GET_MODE (to_rtx))))
+             else if (store_field_updates_msb_p (bitpos, bitsize, to_rtx))
                {
                  scalar_int_mode imode = subreg_unpromoted_mode (to_rtx);
                  scalar_int_mode omode = subreg_promoted_mode (to_rtx);

diff --git a/gcc/rtlanal.cc b/gcc/rtlanal.cc
index 27349a0a74f55c437c67e0489536557bb002a91c..37df8d33e8917a428e38b3419d76d9f188b50552 100644 (file)
--- a/gcc/rtlanal.cc
+++ b/gcc/rtlanal.cc
@@ -6200,6 +6200,17 @@ truncated_to_mode (machine_mode mode, const_rtx x)
   if (REG_P (x) && rtl_hooks.reg_truncated_to_mode (mode, x))
     return true;
 
+  /* This explicit TRUNCATE may be needed on targets that require
+     MODE to be suitably extended when stored in X.  Targets such as
+     mips64 use (sign_extend:DI (truncate:SI (reg:DI x))) to perform
+     an explicit extension, avoiding use of (subreg:SI (reg:DI x))
+     which is assumed to already be extended.  */
+  scalar_int_mode imode, omode;
+  if (is_a <scalar_int_mode> (mode, &imode)
+      && is_a <scalar_int_mode> (GET_MODE (x), &omode)
+      && targetm.mode_rep_extended (imode, omode) != UNKNOWN)
+    return false;
+
   /* See if we already satisfy the requirements of MODE.  If yes we
      can just switch to MODE.  */
   if (num_sign_bit_copies_in_rep[GET_MODE (x)][mode]

diff --git a/gcc/testsuite/gcc.target/mips/pr118608-1.c b/gcc/testsuite/gcc.target/mips/pr118608-1.c
new file mode 100644 (file)
index 0000000..e7384d7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/pr118608-1.c
@@ -0,0 +1,45 @@
+/* { dg-do run } */
+/* { dg-options "-mabi=64 -O2" } */
+
+#define COUNT 10
+
+typedef unsigned short u16;
+typedef unsigned int   u32;
+
+typedef struct NeedleAddress
+{
+  u16   nId;
+  u16   mId;
+} NeedleAddress;
+
+u32 __attribute__ ((noinline)) prepareNeedle(const u16 upper, const u16 lower)
+{
+    u32 needleAddress = 0;
+    NeedleAddress *const addr = (NeedleAddress*)(&needleAddress);
+    addr->mId = upper;
+    addr->nId = lower;
+    return needleAddress;
+}
+
+const u32* __attribute__ ((noinline)) findNeedle(const u32 needle, const u32* begin, const u32* end)
+{
+    while ( begin != end && needle != *begin )
+    {
+        ++begin;
+    }
+    return begin;
+}
+
+int main()
+{
+    u32 needle = prepareNeedle(0xDCBA, 0xABCD);
+
+    u32 haystack[COUNT] = {};
+    for (int i = 0; i < COUNT; i++)
+        haystack[i] = needle;
+
+    const u32* result = findNeedle(needle, haystack, haystack + COUNT);
+    if (result == haystack + COUNT)
+        __builtin_abort ();
+    return 0;
+}

diff --git a/gcc/testsuite/gcc.target/mips/pr118608-2.c b/gcc/testsuite/gcc.target/mips/pr118608-2.c
new file mode 100644 (file)
index 0000000..a5bcfda
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/pr118608-2.c
@@ -0,0 +1,45 @@
+/* { dg-do run } */
+/* { dg-options "-mabi=64 -Os" } */
+
+#define COUNT 10
+
+typedef unsigned short u16;
+typedef unsigned int   u32;
+
+typedef struct NeedleAddress
+{
+  u16   nId;
+  u16   mId;
+} NeedleAddress;
+
+u32 __attribute__ ((noinline)) prepareNeedle(const u16 upper, const u16 lower)
+{
+    u32 needleAddress = 0;
+    NeedleAddress *const addr = (NeedleAddress*)(&needleAddress);
+    addr->mId = upper;
+    addr->nId = lower;
+    return needleAddress;
+}
+
+const u32* __attribute__ ((noinline)) findNeedle(const u32 needle, const u32* begin, const u32* end)
+{
+    while ( begin != end && needle != *begin )
+    {
+        ++begin;
+    }
+    return begin;
+}
+
+int main()
+{
+    u32 needle = prepareNeedle(0xDCBA, 0xABCD);
+
+    u32 haystack[COUNT] = {};
+    for (int i = 0; i < COUNT; i++)
+        haystack[i] = needle;
+
+    const u32* result = findNeedle(needle, haystack, haystack + COUNT);
+    if (result == haystack + COUNT)
+        __builtin_abort ();
+    return 0;
+}

diff --git a/gcc/testsuite/gcc.target/mips/pr118608-3.c b/gcc/testsuite/gcc.target/mips/pr118608-3.c
new file mode 100644 (file)
index 0000000..f017e4e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/pr118608-3.c
@@ -0,0 +1,45 @@
+/* { dg-do compile } */
+/* { dg-options "-mabi=64 -O2 -march=octeon2" } */
+
+#define COUNT 10
+
+typedef unsigned short u16;
+typedef unsigned int   u32;
+
+typedef struct NeedleAddress
+{
+  u16   nId;
+  u16   mId;
+} NeedleAddress;
+
+u32 __attribute__ ((noinline)) prepareNeedle(const u16 upper, const u16 lower)
+{
+    u32 needleAddress = 0;
+    NeedleAddress *const addr = (NeedleAddress*)(&needleAddress);
+    addr->mId = upper;
+    addr->nId = lower;
+    return needleAddress;
+}
+
+const u32* __attribute__ ((noinline)) findNeedle(const u32 needle, const u32* begin, const u32* end)
+{
+    while ( begin != end && needle != *begin )
+    {
+        ++begin;
+    }
+    return begin;
+}
+
+int main()
+{
+    u32 needle = prepareNeedle(0xDCBA, 0xABCD);
+
+    u32 haystack[COUNT] = {};
+    for (int i = 0; i < COUNT; i++)
+        haystack[i] = needle;
+
+    const u32* result = findNeedle(needle, haystack, haystack + COUNT);
+    if (result == haystack + COUNT)
+        __builtin_abort ();
+    return 0;
+}

diff --git a/gcc/testsuite/gcc.target/mips/pr118608-4.c b/gcc/testsuite/gcc.target/mips/pr118608-4.c
new file mode 100644 (file)
index 0000000..4c96e37
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/pr118608-4.c
@@ -0,0 +1,45 @@
+/* { dg-do compile } */
+/* { dg-options "-mabi=64 -Os -march=octeon2" } */
+
+#define COUNT 10
+
+typedef unsigned short u16;
+typedef unsigned int   u32;
+
+typedef struct NeedleAddress
+{
+  u16   nId;
+  u16   mId;
+} NeedleAddress;
+
+u32 __attribute__ ((noinline)) prepareNeedle(const u16 upper, const u16 lower)
+{
+    u32 needleAddress = 0;
+    NeedleAddress *const addr = (NeedleAddress*)(&needleAddress);
+    addr->mId = upper;
+    addr->nId = lower;
+    return needleAddress;
+}
+
+const u32* __attribute__ ((noinline)) findNeedle(const u32 needle, const u32* begin, const u32* end)
+{
+    while ( begin != end && needle != *begin )
+    {
+        ++begin;
+    }
+    return begin;
+}
+
+int main()
+{
+    u32 needle = prepareNeedle(0xDCBA, 0xABCD);
+
+    u32 haystack[COUNT] = {};
+    for (int i = 0; i < COUNT; i++)
+        haystack[i] = needle;
+
+    const u32* result = findNeedle(needle, haystack, haystack + COUNT);
+    if (result == haystack + COUNT)
+        __builtin_abort ();
+    return 0;
+}