Bug 81464 - [8 Regression] ICE in expand_omp_for_static_chunk, at omp-expand.c:4236
Summary: [8 Regression] ICE in expand_omp_for_static_chunk, at omp-expand.c:4236
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 8.0
: P3 normal
Target Milestone: 8.0
Assignee: Not yet assigned to anyone
URL:
Keywords: ice-on-valid-code, patch
Depends on:
Blocks:
 
Reported: 2017-07-17 08:02 UTC by Martin Liška
Modified: 2017-07-19 06:26 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-07-17 00:00:00


Attachments
Tentative patch (684 bytes, patch)
2017-07-18 08:29 UTC, Tom de Vries
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Liška 2017-07-17 08:02:48 UTC
Starting from r247495, we ICE on:

$ gcc  /home/marxin/Programming/gcc/gcc/testsuite/gfortran.dg/matmul_15.f90 -O1 --param parloops-chunk-size=2 -ftree-parallelize-loops=2
during GIMPLE pass: ompexpssa
/home/marxin/Programming/gcc/gcc/testsuite/gfortran.dg/matmul_15.f90:28:0:

            sm = sm + sum(c)
 
internal compiler error: in expand_omp_for_static_chunk, at omp-expand.c:4236
0xacdfd4 expand_omp_for_static_chunk
	../../gcc/omp-expand.c:4236
0xad70b6 expand_omp_for
	../../gcc/omp-expand.c:5840
0xad845a expand_omp
	../../gcc/omp-expand.c:7903
0xad8666 expand_omp
	../../gcc/omp-expand.c:7889
0xadae3d execute_expand_omp
	../../gcc/omp-expand.c:8127
Comment 1 Jakub Jelinek 2017-07-17 08:29:55 UTC
This is when searching for a PHI for vop .MEM_NNN, but apparently there is no such PHI.  I'm afraid I'm not familiar with this part of code to know why it is trying to do that.
Comment 2 Jakub Jelinek 2017-07-17 08:33:55 UTC
CCing Tom as the author of r227437.
Comment 3 Tom de Vries 2017-07-17 15:47:30 UTC
Minimal example:
...
program main
  implicit none
  real, dimension(:,:),allocatable :: a, b, c
  real :: sm

  allocate (a(2,2), b(2,2), c(2,2))

  call random_number(a)
  call random_number(b)

  c = matmul(a,b)
  sm = sum(c)

  deallocate(a,b,c)

end program main
...

The assert happens in expand_omp_for_static_chunk when trying to expand a
region from bb71 to bb72:
...
(gdb) p region.entry.index
$1 = 71
(gdb) p region.exit.index
$2 = 72
...

In other words:
...
;;   basic block 71, loop depth 0, freq 48, maybe hot
;;    prev block 69, next block 67, flags: (NEW)
;;    pred:       69 [always]  (FALLTHRU)
  #pragma omp for schedule(static,2)
  for (ivtmp_87 = 0; ivtmp_87 < _144; ivtmp_87 =  + 1)
;;    succ:       67 [always]  (FALLTHRU)
;;                72 [never (guessed)]

   ...

;;   basic block 72, loop depth 0, freq 47, maybe hot
;;   Invalid sum of incoming frequencies 269, should be 47
;;    prev block 63, next block 70, flags: (NEW)
;;    pred:       46 [always]  (FALLTHRU)
;;                71 [never (guessed)] 
  # .MEM_88 = PHI <.MEM_86(46), .MEM_86(71)>
  #pragma omp return(nowait)
;;    succ:       70 [always]  (FALLTHRU)
...

When we're ICE-ing, we're executing a bit:
...
      /* When we redirect the edge from trip_update_bb to iter_part_bb, we                            
         remove arguments of the phi nodes in fin_bb.  We need to create                              
         appropriate phi nodes in iter_part_bb instead.  */
...

When we look at fin_bb, we see indeed that one of the arguments of the phi has been dropped.:
...
(gdb) call debug_bb (fin_bb)
;; basic block 72, loop depth 0, freq 47, maybe hot
;;  prev block 100, next block 70, flags: (NEW)
;;  pred:       98 [never (guessed)]  (FALSE_VALUE)
# .MEM_88 = PHI <.MEM_86(98)>
;;  succ:       70 [always]  (FALLTHRU)
...

But since the arguments were equal anyway, this is fine, and there's no need to do anything.

Tentative patch:
...
diff --git a/gcc/omp-expand.c b/gcc/omp-expand.c
index 929c530..089bffc 100644
--- a/gcc/omp-expand.c
+++ b/gcc/omp-expand.c
@@ -4206,6 +4206,10 @@ expand_omp_for_static_chunk (struct omp_region *region,
          source_location locus;
 
          phi = psi.phi ();
+         if (operand_equal_p (gimple_phi_arg_def (phi, 0),
+                              gimple_phi_arg_def (phi, 1), 0))
+             continue;
+
          t = gimple_phi_result (phi);
          gcc_assert (t == redirect_edge_var_map_result (vm));
 
...
Comment 4 Tom de Vries 2017-07-18 08:28:20 UTC
(In reply to Tom de Vries from comment #3)
> Tentative patch:
> ...
> diff --git a/gcc/omp-expand.c b/gcc/omp-expand.c
> index 929c530..089bffc 100644
> --- a/gcc/omp-expand.c
> +++ b/gcc/omp-expand.c
> @@ -4206,6 +4206,10 @@ expand_omp_for_static_chunk (struct omp_region
> *region,
>           source_location locus;
>  
>           phi = psi.phi ();
> +         if (operand_equal_p (gimple_phi_arg_def (phi, 0),
> +                              gimple_phi_arg_def (phi, 1), 0))
> +             continue;
> +
>           t = gimple_phi_result (phi);
>           gcc_assert (t == redirect_edge_var_map_result (vm));
>  
> ...

As it turns out, at the point of the operand_equal_p test, we already lost the second argument of the phi.

I thought that this would trigger an assert, but apparently we don't have this:
...
@@ -4379,6 +4379,7 @@ phi_nodes_ptr (basic_block bb)
 static inline tree
 gimple_phi_arg_def (gimple *gs, size_t index)
 {
+  gcc_gimple_checking_assert (index < gimple_phi_num_args(gs));
   return gimple_phi_arg (gs, index)->def;
 }
 
...

Using redirect_edge_var_map_def instead fixes this.
Comment 5 Tom de Vries 2017-07-18 08:29:34 UTC
Created attachment 41779 [details]
Tentative patch
Comment 7 Tom de Vries 2017-07-18 19:26:00 UTC
Patch with test-case committed, marking resolved fixed.
Comment 8 Tom de Vries 2017-07-19 06:26:40 UTC
Author: vries
Date: Wed Jul 19 06:25:40 2017
New Revision: 250335

URL: https://gcc.gnu.org/viewcvs?rev=250335&root=gcc&view=rev
Log:
Handle equal-argument loop exit phi in expand_omp_for_static_chunk

2017-07-18  Tom de Vries  <tom@codesourcery.com>

	PR middle-end/81464
	* omp-expand.c (expand_omp_for_static_chunk): Handle equal-argument loop
	exit phi.

	* gfortran.dg/pr81464.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/pr81464.f90
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/omp-expand.c
    trunk/gcc/testsuite/ChangeLog