https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114284

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
+propagating insn 6 into insn 8, replacing:
+(parallel [
+        (set (reg:SI 114 [ <retval> ])
+            (sign_extend:SI (subreg:HI (reg:SI 117 [ x ]) 0)))
+        (clobber (scratch:SI))
+    ])
+successfully matched this instruction to thumb1_extendhisi2:
+(parallel [
+        (set (reg:SI 114 [ <retval> ])
+            (sign_extend:SI (mem/v/c:HI (reg/f:SI 115) [1 x+0 S2 A16])))
+        (clobber (scratch:SI))
+    ])
I believe before that change we'd never propagate MEM/v, classify_result even
correctly explains why:
     4) Creating new (mem/v)s is not correct, since DCE will not remove the old
        ones.
But now that fwprop just ignores that or takes it as a slight hint, we get the
invalid change.
I'd actually say all the 4 reasons why it shouldn't be propagating MEMs should
result in don't actually propagate it rather than just a mere hint:
  /* Allow (subreg (mem)) -> (mem) simplifications with the following
     exceptions:
     1) Propagating (mem)s into multiple uses is not profitable.
     2) Propagating (mem)s across EBBs may not be profitable if the source EBB
        runs less frequently.
     3) Propagating (mem)s into paradoxical (subreg)s is not profitable.
     4) Creating new (mem/v)s is not correct, since DCE will not remove the old
        ones.  */
and punt maybe also on propagating any other MEMs into insns.
Though, when check_mem is called, it is called only on the MEMs which are being
propagated, so not sure how to actually check it in there.
Doing
--- a/gcc/fwprop.cc
+++ b/gcc/fwprop.cc
@@ -211,6 +211,11 @@ fwprop_propagation::fwprop_propagation (insn_info
*use_insn,
 bool
 fwprop_propagation::check_mem (int old_num_changes, rtx mem)
 {
+  if (MEM_VOLATILE_P (mem))
+    {
+      failure_reason = "would propagate volatile MEM";
+      return false;
+    }
   if (!memory_address_addr_space_p (GET_MODE (mem), XEXP (mem, 0),
                                    MEM_ADDR_SPACE (mem)))
     {
doesn't really help, so perhaps I misunderstand what is check_mem used for.

Reply via email to