[Bug tree-optimization/94949] New: [11 Regression] Wrong code for couple of SPEC benchmarks since r11-39-gf9e1ea10e657af9f

[Bug tree-optimization/94949] New: [11 Regression] Wrong code for couple of SPEC benchmarks since r11-39-gf9e1ea10e657af9f

[marxin at gcc dot gnu.org]

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

            Bug ID: 94949
           Summary: [11 Regression] Wrong code for couple of SPEC
                    benchmarks since r11-39-gf9e1ea10e657af9f
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: marxin at gcc dot gnu.org
                CC: rguenth at gcc dot gnu.org
  Target Milestone: ---

Since the revision the following benchmarks fail:
403.gcc, 416.gamess, 445.gobmk
all with: -Ofast -march=skylake

[Bug tree-optimization/94949] [11 Regression] Wrong code for couple of SPEC benchmarks since r11-39-gf9e1ea10e657af9f

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |11.0
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2020-05-05
           Priority|P3                          |P1
     Ever confirmed|0                           |1
      Known to work|                            |10.0

[Bug tree-optimization/94949] [11 Regression] Wrong code for couple of SPEC benchmarks since r11-39-gf9e1ea10e657af9f

[marxin at gcc dot gnu.org]

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

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Easiest to reproduce:
runspec --config=spec2006 --size=test --iterations=1  --no-reportable --action
run --tune=peak 403.gcc -D
...
****************************************
Contents of cccp.err
****************************************
cccp.in:820: internal error: 7


I'll compare binaries before and after the revision.

[Bug tree-optimization/94949] [11 Regression] Wrong code for couple of SPEC benchmarks since r11-39-gf9e1ea10e657af9f

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|10.0                        |11.0
   Target Milestone|---                         |11.0

[Bug tree-optimization/94949] [11 Regression] Wrong code for couple of SPEC benchmarks since r11-39-gf9e1ea10e657af9f

[marxin at gcc dot gnu.org]

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

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
objfolderdiff.py /tmp/good/ /tmp/bad   
    19/   155: c-typeck.o: different
    61/   155: flow.o: different
    95/   155: loop.o: different
   109/   155: real.o: different
   124/   155: sched-deps.o: different
   142/   155: tree.o: different
   145/   155: varasm.o: different

[Bug tree-optimization/94949] [11 Regression] Wrong code for couple of SPEC benchmarks since r11-39-gf9e1ea10e657af9f

[marxin at gcc dot gnu.org]

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

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
>     19/   155: c-typeck.o: different

This one is miscompiled.

[Bug tree-optimization/94949] [11 Regression] Wrong code for couple of SPEC benchmarks since r11-39-gf9e1ea10e657af9f

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
OK, so I guess with -Ofast (-fallow-store-data-races!) we cannot do the
optimization of eliding the loads.

diff --git a/gcc/tree-ssa-loop-im.c b/gcc/tree-ssa-loop-im.c
index 18e5c18c17e..554dd4be5bb 100644
--- a/gcc/tree-ssa-loop-im.c
+++ b/gcc/tree-ssa-loop-im.c
@@ -2128,9 +2128,9 @@ execute_sm (class loop *loop, vec<edge> exits, im_mem_ref
*ref)
   fmt_data.orig_loop = loop;
   for_each_index (&ref->mem.ref, force_move_till, &fmt_data);

+  bool always_stored = ref_always_accessed_p (loop, ref, true);
   if (bb_in_transaction (loop_preheader_edge (loop)->src)
-      || (! flag_store_data_races
-         && ! ref_always_accessed_p (loop, ref, true)))
+      || (! flag_store_data_races && ! always_stored))
     multi_threaded_model_p = true;

   if (multi_threaded_model_p)
@@ -2145,8 +2145,10 @@ execute_sm (class loop *loop, vec<edge> exits,
im_mem_ref *ref)

   /* Avoid doing a load if there was no load of the ref in the loop.
      Esp. when the ref is not always stored we cannot optimize it
-     away later.  */
-  if (ref->loaded && bitmap_bit_p (ref->loaded, loop->num))
+     away later.  But when it is not always stored we must use a conditional
+     store then.  */
+  if ((!always_stored && !multi_threaded_model_p)
+      || (ref->loaded && bitmap_bit_p (ref->loaded, loop->num)))
     {
       load = gimple_build_assign (tmp_var, unshare_expr (ref->mem.ref));
       lim_data = init_lim_data (load);

[Bug tree-optimization/94949] [11 Regression] Wrong code for couple of SPEC benchmarks since r11-39-gf9e1ea10e657af9f

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Mine.

[Bug tree-optimization/94949] [11 Regression] Wrong code for couple of SPEC benchmarks since r11-39-gf9e1ea10e657af9f

[marxin at gcc dot gnu.org]

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

--- Comment #6 from Martin Liška <marxin at gcc dot gnu.org> ---
The patch works for me!

[Bug tree-optimization/94949] [11 Regression] Wrong code for couple of SPEC benchmarks since r11-39-gf9e1ea10e657af9f

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:0424a5ece5307cc22bbc0fe97edf4707d7a798ed

commit r11-67-g0424a5ece5307cc22bbc0fe97edf4707d7a798ed
Author: Richard Biener <rguenther@suse.de>
Date:   Tue May 5 11:00:09 2020 +0200

    tree-optimization/94949 - fix load eliding in SM

    This fixes the case of not using the multithreaded model when
    only conditionally storing to the destination.  We cannot elide
    the load in this case.

    2020-05-05  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/94949
            * tree-ssa-loop-im.c (execute_sm): Check whether we use
            the multithreaded model or always compute the stored value
            before eliding a load.

            * gcc.dg/torture/pr94949.c: New testcase.

[Bug tree-optimization/94949] [11 Regression] Wrong code for couple of SPEC benchmarks since r11-39-gf9e1ea10e657af9f

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.