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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rsandifo at gcc dot gnu.org

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
So even with -ffuse-ops-with-volatile-access this isn't fixed, likely because
combine doesn't work cross-[extended]-BB.  I'm not sure which pass would,
IIRC fwprop but fwprop_addr does

          struct loop *loop = insn->bb ()->cfg_bb ()->loop_father;
          /* The outermost loop is not really a loop.  */
          if (loop == NULL || loop_outer (loop) == NULL)
            {
              if (forward_propagate_into (use, fwprop_addr_p))
                return true;
            }
          else if (fwprop_addr_p)
            {
              if (forward_propagate_into (use, false))
^^^^
                return true;

and

  /* Allow propagations into a loop only for reg-to-reg copies, since
     replacing one register by another shouldn't increase the cost.
     Propagations from inner loop to outer loop should also be ok.  */
  struct loop *def_loop = def_insn->bb ()->cfg_bb ()->loop_father;
  struct loop *use_loop = use->bb ()->cfg_bb ()->loop_father;
  if ((reg_prop_only
       || (def_loop != use_loop
           && !flow_loop_nested_p (use_loop, def_loop)))
      && (!reg_single_def_p (dest) || !reg_single_def_p (src)))
    return false;

so we fail here since 'src' isn't single-def and the loops are nested.
'src' is

(plus:DI (reg/f:DI 98 [ _1 ])
    (const_int 8 [0x8]))

this lacks (target specific?) heuristics that a constant offset can be
propagated for free(?).  Short-cutting the above check allows the propagation
to happen in fwprop2:

propagating insn 7 into insn 11, replacing:
(set (reg:DI 99 [ _3 ])
    (mem/v:DI (reg/f:DI 100 [ _9 ]) [-1  S8 A64]))
successfully matched this instruction to *movdi_internal:
(set (reg:DI 99 [ _3 ])
    (mem/v:DI (plus:DI (reg/f:DI 98 [ _1 ])
            (const_int 8 [0x8])) [-1  S8 A64]))
rescanning insn with uid = 11.
updating insn 11 in-place

I wonder if we instead of ruling out reg_single_def_p (src) we can use
costs to determine whether the transform is worthwhile?  Aka if the
insn we propagate into is in a loop require equal or smaller cost.
We could make sure likely_profitable_p () is false and hope for
change_is_worthwhile?  How costing works in this pass isn't clear to me.

On the GIMPLE level the atomic ops are function calls, one is even unused,
but there's no way for IVOPTs to merge the address.

The following makes fwprop2 do the propagation:

diff --git a/gcc/fwprop.cc b/gcc/fwprop.cc
index 59df7cad0de..6ae5a6dc448 100644
--- a/gcc/fwprop.cc
+++ b/gcc/fwprop.cc
@@ -874,7 +874,7 @@ forward_propagate_into (use_info *use, bool reg_prop_only =
false)
   if ((reg_prop_only
        || (def_loop != use_loop
           && !flow_loop_nested_p (use_loop, def_loop)))
-      && (!reg_single_def_p (dest) || !reg_single_def_p (src)))
+      && !reg_single_def_p (dest))
     return false;

   /* Don't substitute into a non-local goto, this confuses CFG.  */

Reply via email to