[Bug tree-optimization/80304] [7 Regression] Wrong result with do concurrent

[tkoenig at gcc dot gnu.org]

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

Thomas Koenig  changed:

   What|Removed |Added

   Priority|P4  |P3
  Component|fortran |tree-optimization

--- Comment #16 from Thomas Koenig  ---
Not a Fortran bug, then.

[Bug tree-optimization/80304] [7 Regression] Wrong result with do concurrent

[bernd.edlinger at hotmail dot de]

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

Bernd Edlinger  changed:

   What|Removed |Added

 CC||bernd.edlinger at hotmail dot 
de

--- Comment #17 from Bernd Edlinger  ---
(In reply to Thomas Koenig from comment #15)
> Also fails on powerpc64-unknown-linux-gnu.
> 
> r239317 works, r239326 fails, and also appears to be the only
> patch in the vicinity that does anything about loops.
> 
> Could this be the cause?
> 
> Author: edlinger
> Date: Fri Aug 12 19:30:39 2016
> New Revision: 239426
> 
> URL: https://gcc.gnu.org/viewcvs?rev=239426&root=gcc&view=rev
> Log:
> 2016-08-12  Bernd Edlinger  
> 
> PR tree-optimization/71083
> * tree-predcom.c (ref_at_iteration): Use a COMPONENT_REF for the
> bitfield access when possible.
> 
> Modified:
> trunk/gcc/ChangeLog
> trunk/gcc/tree-predcom.c

I really doubt that, because that optimization pass is only used at -O3.
What I can see is that at -O1 -fno-inline 
the *.optimized simply does not initialize i:

sub ()
{
  unsigned int ivtmp.38;
  integer(kind=4) j.28;
  real(kind=4) D.3534;
  real(kind=4) D.3527;
  struct __st_parameter_dt dt_parm.4;
  integer(kind=4) j.1;
  integer(kind=4) i.0;
  integer(kind=8) _2;
  integer(kind=8) _3;
  real(kind=4) _7;
  real(kind=4) _8;
  real(kind=4) _13;
  real(kind=4) _15;
  real(kind=4) _24;
  real(kind=4) val.8_25;
  struct FRAME.test * _26;
  sizetype _39;
  real(kind=4) val.5_48;
  real(kind=4) val.8_55;
  real(kind=4) val.5_71;
  real(kind=4) _72;
  sizetype _76;
  real(kind=4) _101;
  real(kind=4) _103;
  real(kind=4) _113;
  real(kind=4) val.8_114;
  real(kind=4) _116;
  real(kind=4) val.8_117;
  real(kind=4) _125;
  real(kind=4) val.5_126;
  real(kind=4) _128;
  real(kind=4) val.5_129;

   [6.83%]:
  CHAIN.19_29(D)->a = {};
  CHAIN.19_29(D)->b = {};
  j.1 = 0;

   [13.67%]:
  # ivtmp.38_68 = PHI 
  j.28_86 = (integer(kind=4)) ivtmp.38_68;
  _2 = (integer(kind=8)) j.28_86;
  _3 = _2 * 2;
  _101 = add (&i.0);

so it is clear why that can not work.
at .gimple it was initialized:


sub ()
{
  logical(kind=1) D.3551;
  logical(kind=1) D.3554;

  a = {};
  b = {};
  {
integer(kind=4) i.0;
integer(kind=4) j.1;

try
  {
{
  integer(kind=4) count.2;
  integer(kind=4) count.3;

  j.1 = 0;
  count.3 = 2;
  :
  _1 = count.3 <= 0;
  D.3551 = ANNOTATE (_1, 0);
  if (D.3551 != 0) goto L.3; else goto ;
  :
  i.0 = 0;
  count.2 = 2;
  :
  _2 = count.2 <= 0;
  D.3554 = ANNOTATE (_2, 0);
  if (D.3554 != 0) goto L.2; else goto ;
  :
  j.12_3 = j.1;
  _4 = (integer(kind=8)) j.12_3;
  _5 = _4 * 2;
  i.13_6 = i.0;
  _7 = (integer(kind=8)) i.13_6;
  _8 = _5 + _7;
  _9 = add (&i.0);

[Bug tree-optimization/80304] [7 Regression] Wrong result with do concurrent

[dominiq at lps dot ens.fr]

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

--- Comment #18 from Dominique d'Humieres  ---
> r239317 works, r239326 fails, and also appears to be the only
> patch in the vicinity that does anything about loops.
>
> Could this be the cause?
> 
> Author: edlinger
> Date: Fri Aug 12 19:30:39 2016
> New Revision: 239426

Actually it is r239326

Fix PR tree-optimization/71734

2016-08-10  Yuri Rumyantsev  

PR tree-optimization/71734
* tree-ssa-loop-im.c (ref_indep_loop_p): Add new argument
REF_LOOP, invoke ref_indep_loop_p_1.
(outermost_indep_loop): Pass LOOP argumnet where REF was defined
to ref_indep_loop_p.
(ref_indep_loop_p_1): Fix commentary, add argument REF_LOOP,
combine it with ref_indep_lopp_p_2, update SAFELEN if only REF
is inside LOOP, do not cache dpendence value for loops with
non-zero SAFELEN.
(ref_indep_loop_p_2): Delete function.
(can_sm_ref_p): Pass LOOP as additional argument to
ref_indep_loop_p.

Reverting the commit fixes the tests in this PR and in pr80305 (then as
expected libgomp.fortran/pr71734-1.f90  and libgomp.fortran/pr71734-2.f90
fail).

[Bug tree-optimization/80304] [7 Regression] Wrong result with do concurrent

[dominiq at lps dot ens.fr]

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

Dominique d'Humieres  changed:

   What|Removed |Added

 CC||ysrumyan at gmail dot com

--- Comment #19 from Dominique d'Humieres  ---
CCed Yuri Rumyantsev.

[Bug tree-optimization/80304] [7 Regression] Wrong result with do concurrent

[tkoenig at gcc dot gnu.org]

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

Thomas Koenig  changed:

   What|Removed |Added

   Severity|normal  |major

[Bug tree-optimization/80304] [7 Regression] Wrong result with do concurrent

[tkoenig at gcc dot gnu.org]

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

Thomas Koenig  changed:

   What|Removed |Added

   Target Milestone|--- |7.0

[Bug tree-optimization/80304] [7 Regression] Wrong result with do concurrent

[tkoenig at gcc dot gnu.org]

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

Thomas Koenig  changed:

   What|Removed |Added

 CC||rguenth at gcc dot gnu.org

--- Comment #20 from Thomas Koenig  ---
Hi Richard,

could you reclassify this bug?  I have reset the priority to P3
because it appears to be a middle-end bug which just happens
to be exposed by the Fortran front end.

If it is not fixed in time for the release, we should disable the
ivdep annotation of DO CONCURRENT with the patch

Index: trans-stmt.c
===
--- trans-stmt.c(Revision 246743)
+++ trans-stmt.c(Arbeitskopie)
@@ -3397,11 +3397,7 @@ gfc_trans_forall_loop (forall_info *forall_tmp, tr
   /* The exit condition.  */
   cond = fold_build2_loc (input_location, LE_EXPR, boolean_type_node,
  count, build_int_cst (TREE_TYPE (count), 0));
-  if (forall_tmp->do_concurrent)
-   cond = build2 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
-  build_int_cst (integer_type_node,
- annot_expr_ivdep_kind));
-
+  
   tmp = build1_v (GOTO_EXPR, exit_label);
   tmp = fold_build3_loc (input_location, COND_EXPR, void_type_node,
 cond, tmp, build_empty_stmt (input_location));

[Bug tree-optimization/80304] [7 Regression] Wrong result with do concurrent

[rguenth at gcc dot gnu.org]

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

Richard Biener  changed:

   What|Removed |Added

   Priority|P3  |P1
 Status|NEW |ASSIGNED
   Assignee|unassigned at gcc dot gnu.org  |rguenth at gcc dot 
gnu.org

--- Comment #21 from Richard Biener  ---
I will have a look, hopefully tomorrow.

[Bug tree-optimization/80304] [7 Regression] Wrong result with do concurrent

[rguenth at gcc dot gnu.org]

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

--- Comment #22 from Chinoune  ---
Created attachment 41163
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=41163&action=edit
the test program in c

This is an equivalent program written in c.

--- Comment #23 from Richard Biener  ---
The issue is that safelen (and IVDEP, as that uses safelen) is quite aggressive
and does not only apply to what is "memory" in the language specific sense but
also to what ends up as memory to the middle-end.  In this case dependencies
of the not inlined add2 calls which get i and j by reference are not accounted
for which makes i and j suitable to store-motion.

Now all would be fine but the call arguments are not updated by store-motion.

So using IVDEP on loops with calls with reference passing is dangerous.

There's still one bug in LIM, namely it fails to recurse to handle
UNANALYZABLE_MEM_ID for safelen.  The following testcase fixes the
last testcase.

Index: gcc/tree-ssa-loop-im.c
===
--- gcc/tree-ssa-loop-im.c  (revision 246797)
+++ gcc/tree-ssa-loop-im.c  (working copy)
@@ -2145,9 +2145,21 @@ ref_indep_loop_p_1 (int safelen, struct
  fprintf (dump_file, "\n");
}

+  /* We need to recurse to properly handle UNANALYZABLE_MEM_ID.  */
+  struct loop *inner = loop->inner;
+  while (inner)
+   {
+ if (!ref_indep_loop_p_1 (safelen, inner, ref, stored_p, ref_loop))
+   {
+ indep_p = false;
+ break;
+   }
+ inner = inner->next;
+   }
+
   /* Avoid caching here as safelen depends on context and refs
  are shared between different contexts.  */
-  return true;
+  return indep_p;
 }
   else
 {

[Bug tree-optimization/80304] [7 Regression] Wrong result with do concurrent

[rguenth at gcc dot gnu.org]

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

Richard Biener  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #25 from Richard Biener  ---
Fixed. (hopefully)

[Bug tree-optimization/80304] [7 Regression] Wrong result with do concurrent

[rguenth at gcc dot gnu.org]

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

--- Comment #24 from Richard Biener  ---
Author: rguenth
Date: Mon Apr 10 11:27:05 2017
New Revision: 246803

URL: https://gcc.gnu.org/viewcvs?rev=246803&root=gcc&view=rev
Log:
2017-04-10  Richard Biener  

PR tree-optimization/80304
* tree-ssa-loop-im.c (ref_indep_loop_p_1): Also recurse
for safelen.

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

Added:
trunk/gcc/testsuite/gcc.dg/torture/pr80304.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/testsuite/ChangeLog
trunk/gcc/tree-ssa-loop-im.c

[Bug tree-optimization/80304] [7 Regression] Wrong result with do concurrent

[tkoenig at gcc dot gnu.org]

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

--- Comment #26 from Thomas Koenig  ---
Author: tkoenig
Date: Mon Apr 10 20:40:48 2017
New Revision: 246824

URL: https://gcc.gnu.org/viewcvs?rev=246824&root=gcc&view=rev
Log:
2017-04-10  Thomas Koenig  

PR tree-optimization/80304
* gfortran.dg/do_concurrent_4.f90:  New test case.


Added:
trunk/gcc/testsuite/gfortran.dg/do_concurrent_4.f90
Modified:
trunk/gcc/testsuite/ChangeLog

[Bug tree-optimization/80304] [7 Regression] Wrong result with do concurrent

[tkoenig at gcc dot gnu.org]

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

--- Comment #27 from Thomas Koenig  ---
The bug is indeed fixed.  Thanks a lot for the quick fix!

I have also committed a Fortran test case (just to be sure).

Regards

Thomas