This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Fix PR rtl-optimization/84071


This is a regression present on mainline and 7 branch for big-endian ARM in 
the form of a wrong elimination of zero-extension after sign-extended load.

The problem is that reg_nonzero_bits_for_combine returns 0xffff for the 
register (reg:HI 121) when queried for SImode after:

(insn 10 7 11 2 (set (subreg:SI (reg:HI 121) 0)
        (sign_extend:SI (mem/c:HI (plus:SI (reg/f:SI 103 afp)
                    (const_int 4 [0x4])) [1 u+0 S2 A32]))) 171 
{*arm_extendhisi2_v6}

That's a valid query since the combiner keeps track of nonzero_bits in the 
largest possible mode:

/* Mode used to compute significance in reg_stat[].nonzero_bits.  It is the
   largest integer mode that can fit in HOST_BITS_PER_WIDE_INT.  */

static machine_mode nonzero_bits_mode;

The root cause of the issue is the SUBREG case of record_dead_and_set_regs_1:

  if (REG_P (dest))
    {
      /* If we are setting the whole register, we know its value.  Otherwise
	 show that we don't know the value.  We can handle SUBREG in
	 some cases.  */
      if (GET_CODE (setter) == SET && dest == SET_DEST (setter))
	record_value_for_reg (dest, record_dead_insn, SET_SRC (setter));
      else if (GET_CODE (setter) == SET
	       && GET_CODE (SET_DEST (setter)) == SUBREG
	       && SUBREG_REG (SET_DEST (setter)) == dest
	       && known_le (GET_MODE_PRECISION (GET_MODE (dest)), BITS_PER_WORD)
	       && subreg_lowpart_p (SET_DEST (setter)))
	record_value_for_reg (dest, record_dead_insn,
			      gen_lowpart (GET_MODE (dest),
						       SET_SRC (setter)));
      else
	record_value_for_reg (dest, record_dead_insn, NULL_RTX);
    }

It's very old (r357) and effectively synthesizes a wrong SET operation here 
because it strips the sign-extension around the MEM; now rtlanal.c considers 
that loads are extended according to LOAD_EXTEND_OP on RISC platforms and ARM 
is a ZERO_EXTEND target in this configuration (ARMv4 and above), so the sign-
extension is effectively turned into a zero-extension.

This SUBREG case looks quite questionable, not only for paradoxical SUBREGs on 
RISC platforms, but also for regular SUBREGs, for which it's trying to infer 
information on the whole register from a SET of the lowpart.  But the fix only 
changes the first, problematic case.

Tested on SPARC64/Linux and ARM/EABI, applied on mainline and 7 branch.


2018-01-31  Eric Botcazou  <ebotcazou@adacore.com>

	PR rtl-optimization/84071
	* combine.c (record_dead_and_set_regs_1): Record the source unmodified
	for a paradoxical SUBREG on a WORD_REGISTER_OPERATIONS target.


2018-01-31  Eric Botcazou  <ebotcazou@adacore.com>

	* gcc.c-torture/execute/20180131-1.c: New test.

-- 
Eric Botcazou
Index: combine.c
===================================================================
--- combine.c	(revision 257139)
+++ combine.c	(working copy)
@@ -13245,18 +13245,25 @@ record_dead_and_set_regs_1 (rtx dest, co
   if (REG_P (dest))
     {
       /* If we are setting the whole register, we know its value.  Otherwise
-	 show that we don't know the value.  We can handle SUBREG in
-	 some cases.  */
+	 show that we don't know the value.  We can handle a SUBREG if it's
+	 the low part, but we must be careful with paradoxical SUBREGs on
+	 RISC architectures because we cannot strip e.g. an extension around
+	 a load and record the naked load since the RTL middle-end considers
+	 that the upper bits are defined according to LOAD_EXTEND_OP.  */
       if (GET_CODE (setter) == SET && dest == SET_DEST (setter))
 	record_value_for_reg (dest, record_dead_insn, SET_SRC (setter));
       else if (GET_CODE (setter) == SET
 	       && GET_CODE (SET_DEST (setter)) == SUBREG
 	       && SUBREG_REG (SET_DEST (setter)) == dest
-	       && known_le (GET_MODE_PRECISION (GET_MODE (dest)), BITS_PER_WORD)
+	       && known_le (GET_MODE_PRECISION (GET_MODE (dest)),
+			    BITS_PER_WORD)
 	       && subreg_lowpart_p (SET_DEST (setter)))
 	record_value_for_reg (dest, record_dead_insn,
-			      gen_lowpart (GET_MODE (dest),
-						       SET_SRC (setter)));
+			      WORD_REGISTER_OPERATIONS
+			      && paradoxical_subreg_p (SET_DEST (setter))
+			      ? SET_SRC (setter)
+			      : gen_lowpart (GET_MODE (dest),
+					     SET_SRC (setter)));
       else
 	record_value_for_reg (dest, record_dead_insn, NULL_RTX);
     }
/* PR rtl-optimization/84071 */
/* Reported by Wilco <wilco@gcc.gnu.org> */

extern void abort (void);

typedef union 
{
  signed short ss;
  unsigned short us;
  int x;
} U;

int f(int x, int y, int z, int a, U u) __attribute__((noclone, noinline));

int f(int x, int y, int z, int a, U u)
{
  return (u.ss <= 0) + u.us;
}

int main (void)
{
  U u = { .ss = -1 };

  if (f (0, 0, 0, 0, u) != (1 << sizeof (short) * 8))
    abort ();

  return 0;
}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]