Re: [PATCH v6] Implement new RTL optimizations pass: fold-mem-offsets.

[Jeff Law]

On 10/3/23 05:45, Manolis Tsamis wrote:
This is a new RTL pass that tries to optimize memory offset calculations

+
+/* If INSN is a root memory instruction then compute a potentially new offset
+   for it and test if the resulting instruction is valid.  */
+static void
+do_check_validity (rtx_insn *insn, fold_mem_info *info)
+{
+  rtx mem, reg;
+  HOST_WIDE_INT cur_offset;
+  if (!get_fold_mem_root (insn, &mem, &reg, &cur_offset))
+    return;
+
+  HOST_WIDE_INT new_offset = cur_offset + info->added_offset;
+
+  /* Test if it is valid to change MEM's address offset to NEW_OFFSET.  */
+  int icode = INSN_CODE (insn);
+  rtx mem_addr = XEXP (mem, 0);
+  machine_mode mode = GET_MODE (mem_addr);
+  if (new_offset != 0)
+    XEXP (mem, 0) = gen_rtx_PLUS (mode, reg, gen_int_mode (new_offset, mode));
+  else
+    XEXP (mem, 0) = reg;
+
+  bool illegal = insn_invalid_p (insn, false)
+                || !memory_address_addr_space_p (mode, XEXP (mem, 0),
+                                                 MEM_ADDR_SPACE (mem));
+
+  /* Restore the instruction.  */
+  XEXP (mem, 0) = mem_addr;
+  INSN_CODE (insn) = icode;
+
+  if (illegal)
+    bitmap_ior_into (&cannot_fold_insns, info->fold_insns);
+  else
+    bitmap_ior_into (&candidate_fold_insns, info->fold_insns);
+}
+
So overnight testing with the latest version of your patch triggered a 
fault on the sh3-linux-gnu target with this code at -O2:

enum
{
  _ISspace = ((5) < 8 ? ((1 << (5)) << 8) : ((1 << (5)) >> 8)),
};
extern const unsigned short int **__ctype_b_loc (void)
     __attribute__ ((__nothrow__ )) __attribute__ ((__const__));
void
read_alias_file (const char *fname, int fname_len)
{
      char buf[400];
      char *cp;
      cp = buf;
      while (((*__ctype_b_loc ())[(int) (((unsigned char) cp[0]))] & (unsigned 
short int) _ISspace))
       ++cp;
}



The problem is we need to clear the INSN_CODE before we call recog.  In 
this specific case we had (mem (plus (reg) (offset)) after f-m-o does 
its job, the offset went to zero so we changed the structure of the RTL 
to (mem (reg)).  But we had the old INSN_CODE still in place which 
caused us to reference operands that no longer exist.

A simple INSN_CODE (insn) = -1 before calling_insn_invalid_p is the 
right fix.

jeff