[Bug tree-optimization/49471] New: ICE when -ftree-parallelize-loops is enabled together with -m32 on power7

[Bug tree-optimization/49471] New: ICE when -ftree-parallelize-loops is enabled together with -m32 on power7

[razya at il dot ibm.com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49471

           Summary: ICE when -ftree-parallelize-loops is enabled together
                    with -m32 on power7
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: razya@il.ibm.com


When building cactusADM on igoo, I get an error when autopar is enabled with
-m32
(does not happen for 64!)

This is the command line which is executed:

> /home/razya/gcc-bin/bin/gcc -c -o PUGHReduce/ReductionNormInf.o -DSPEC_CPU -DNDEBUG  -Iinclude -I../include -DCCODE -m32  -ffast-math -O3  -fno-tree-vectorize -fno-vect-cost-model -ftree-parallelize-loops=6         PUGHReduce/ReductionNormInf.c


The source of the failure is at the code generated to expand omp_for pragma (in
tree level).
The expansion of pragma omp_for generates a prolog code which calculates the
particular interval of (the loop's) iterations which the  thread (executing
this code) should execute.
This prolog calculation involves some arithmetic operations, and in particular
MULT and DIV statements which  cause the failures.

so, for example, this is the tree level code generated for the prolog, and the
div and mult operations highlighted in red:


  D.7313_5 = MEM[(struct  *).paral_data_param_1(D)].D.7288; /*  Number of loop
iterations.  */
   D.7316_8 = __builtin_omp_get_num_threads ();
  D.7317_9 = (<unnamed-unsigned:128>) D.7316_8;
  D.7318_10 = __builtin_omp_get_thread_num ();
  D.7319_11 = (<unnamed-unsigned:128>) D.7318_10;
  D.7320_12 = D.7313_5 / D.7317_9;
  D.7321_13 = D.7320_12 * D.7317_9;
  D.7322_14 = D.7321_13 != D.7313_5;
  D.7323_15 = D.7322_14 + D.7320_12;
  ivtmp.575_16 = D.7323_15 * D.7319_11;
  D.7325_17 = ivtmp.575_16 + D.7323_15;
  D.7326_18 = MIN_EXPR <D.7325_17, D.7313_5>;
  if (ivtmp.575_16 >= D.7326_18)
    goto <bb 3>;
  else
    goto <bb 4>;



 when the div expr is  being expanded to RTL code, we fail in executing 
the following expand_binop:
expmed.c:

            quotient = sign_expand_binop (compute_mode,
                                              udiv_optab, sdiv_optab,
                                              op0, op1, target,
                                              unsignedp, OPTAB_LIB_WIDEN);

The call to this function returns NULL (where it shouldn't s far as I
understand).

When I tried removing the div instruction, the mult expr caused an assert
failure 
 in expand_mult() because the following call returned NULL.:

 expmed.c:

  /* This used to use umul_optab if unsigned, but for non-widening multiply
     there is no difference between signed and unsigned.  */
  op0 = expand_binop (mode,
                      ! unsignedp
                      && flag_trapv && (GET_MODE_CLASS(mode) == MODE_INT)
                      ? smulv_optab : smul_optab,
                      op0, op1, target, unsignedp, OPTAB_LIB_WIDEN);
  gcc_assert (op0);

-------------------------------------------------------------------------------

I found that the variables being divided/multiplied are of 128 bit types.
They are created when canonicalize_loop_ivs is called:

tree
canonicalize_loop_ivs (struct loop *loop, tree *nit, bool bump_in_latch)
{
  unsigned precision = TYPE_PRECISION (TREE_TYPE (*nit));   //precision of
number of iterations

  for (psi = gsi_start_phis (loop->header);         
       !gsi_end_p (psi); gsi_next (&psi))
    {
      gimple phi = gsi_stmt (psi);
      tree res = PHI_RESULT (phi);

      if (is_gimple_reg (res) && TYPE_PRECISION (TREE_TYPE (res)) > precision)
        precision = TYPE_PRECISION (TREE_TYPE (res));
    }

  type = lang_hooks.types.type_for_size (precision, 1);    // here precision is
128 

....
}


Note that this is also the case when -m64 is enabled.
The difference is that the type created by lang_hooks for the -m32 case is
<unnamed-unsigned:128>
and for -m64 it is __int128 unsigned, whose arithmetic operations apparently
are handled correctly by the compiler.

[Bug tree-optimization/49471] ICE when -ftree-parallelize-loops is enabled together with -m32 on power7

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49471

--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-06-20 10:22:52 UTC ---
Why is

  D.7313_5 = MEM[(struct  *).paral_data_param_1(D)].D.7288; /*  Number of loop
iterations.  */

of type __int128?  That looks bogus.

[Bug tree-optimization/49471] ICE when -ftree-parallelize-loops is enabled together with -m32 on power7

[razya at il dot ibm.com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49471

--- Comment #2 from razya at il dot ibm.com 2011-06-20 12:31:32 UTC ---
(In reply to comment #1)
> Why is
>   D.7313_5 = MEM[(struct  *).paral_data_param_1(D)].D.7288; /*  Number of loop
> iterations.  */
> of type __int128?  That looks bogus.

the size of 128 was determined according to the precision of the ivs in
canonicalize_loop_ivs:

canonicalize_loop_ivs (struct loop *loop, tree *nit, bool bump_in_latch)
{
  unsigned precision = TYPE_PRECISION (TREE_TYPE (*nit));   
  for (psi = gsi_start_phis (loop->header);         
       !gsi_end_p (psi); gsi_next (&psi))
    {
      gimple phi = gsi_stmt (psi);
      tree res = PHI_RESULT (phi);

      if (is_gimple_reg (res) && TYPE_PRECISION (TREE_TYPE (res)) > precision)
        precision = TYPE_PRECISION (TREE_TYPE (res));
    }

  type = lang_hooks.types.type_for_size (precision, 1); // precision == 128 
  ...
 }

Does it seem that the precision should not determine the new type size, or that 
the precision itself being 128 is strange?

[Bug tree-optimization/49471] ICE when -ftree-parallelize-loops is enabled together with -m32 on power7

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49471

--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-06-20 12:55:10 UTC ---
(In reply to comment #2)
> (In reply to comment #1)
> > Why is
> >   D.7313_5 = MEM[(struct  *).paral_data_param_1(D)].D.7288; /*  Number of loop
> > iterations.  */
> > of type __int128?  That looks bogus.
> 
> the size of 128 was determined according to the precision of the ivs in
> canonicalize_loop_ivs:
> 
> canonicalize_loop_ivs (struct loop *loop, tree *nit, bool bump_in_latch)
> {
>   unsigned precision = TYPE_PRECISION (TREE_TYPE (*nit));   
>   for (psi = gsi_start_phis (loop->header);         
>        !gsi_end_p (psi); gsi_next (&psi))
>     {
>       gimple phi = gsi_stmt (psi);
>       tree res = PHI_RESULT (phi);
> 
>       if (is_gimple_reg (res) && TYPE_PRECISION (TREE_TYPE (res)) > precision)
>         precision = TYPE_PRECISION (TREE_TYPE (res));
>     }
> 
>   type = lang_hooks.types.type_for_size (precision, 1); // precision == 128 
>   ...
>  }
> 
> Does it seem that the precision should not determine the new type size, or that 
> the precision itself being 128 is strange?

Well, autopar seems to introduce this 128 bit type in the first place,
and I wonder why it does that.  And it definitely should avoid doing this.

[Bug tree-optimization/49471] ICE when -ftree-parallelize-loops is enabled together with -m32 on power7

[razya at il dot ibm.com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49471

--- Comment #4 from razya at il dot ibm.com 2011-06-20 14:14:13 UTC ---
(In reply to comment #3)
> (In reply to comment #2)
> > (In reply to comment #1)
> > > Why is
> > >   D.7313_5 = MEM[(struct  *).paral_data_param_1(D)].D.7288; /*  Number of loop
> > > iterations.  */
> > > of type __int128?  That looks bogus.
> > 
> > the size of 128 was determined according to the precision of the ivs in
> > canonicalize_loop_ivs:
> > 
> > canonicalize_loop_ivs (struct loop *loop, tree *nit, bool bump_in_latch)
> > {
> >   unsigned precision = TYPE_PRECISION (TREE_TYPE (*nit));   
> >   for (psi = gsi_start_phis (loop->header);         
> >        !gsi_end_p (psi); gsi_next (&psi))
> >     {
> >       gimple phi = gsi_stmt (psi);
> >       tree res = PHI_RESULT (phi);
> > 
> >       if (is_gimple_reg (res) && TYPE_PRECISION (TREE_TYPE (res)) > precision)
> >         precision = TYPE_PRECISION (TREE_TYPE (res));
> >     }
> > 
> >   type = lang_hooks.types.type_for_size (precision, 1); // precision == 128 
> >   ...
> >  }
> > 
> > Does it seem that the precision should not determine the new type size, or that 
> > the precision itself being 128 is strange?
> Well, autopar seems to introduce this 128 bit type in the first place,
> and I wonder why it does that.  And it definitely should avoid doing this.

What happens is that autopar calls canonicalize_loop_ivs() when it is starting
to change the loop.

Here's a  part of the documentation of canonicalize_loop_ivs(): 

 "  When the IV type precision has to be larger
   than *NIT type precision, *NIT is converted to the larger type, the
   conversion code is inserted before the loop, and *NIT is updated to
   the new definition.  "  

In this case of cactusADM, one of the loop's IVs indeed has a precision of 128,
and therefore a conversion to a type of 128 bit is created.

I checked the precision of the loop's IVs a few passes before autopar, and even
when I disable autopar, and indeed there is an IV that has a type with 128
precision.

[Bug tree-optimization/49471] ICE when -ftree-parallelize-loops is enabled together with -m32 on power7

[razya at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49471

--- Comment #5 from razya at gcc dot gnu.org 2011-07-13 15:06:50 UTC ---
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > (In reply to comment #1)
> > > > Why is
> > > >   D.7313_5 = MEM[(struct  *).paral_data_param_1(D)].D.7288; /*  Number of loop
> > > > iterations.  */
> > > > of type __int128?  That looks bogus.
> > > 
> > > the size of 128 was determined according to the precision of the ivs in
> > > canonicalize_loop_ivs:
> > > 
> > > canonicalize_loop_ivs (struct loop *loop, tree *nit, bool bump_in_latch)
> > > {
> > >   unsigned precision = TYPE_PRECISION (TREE_TYPE (*nit));   
> > >   for (psi = gsi_start_phis (loop->header);         
> > >        !gsi_end_p (psi); gsi_next (&psi))
> > >     {
> > >       gimple phi = gsi_stmt (psi);
> > >       tree res = PHI_RESULT (phi);
> > > 
> > >       if (is_gimple_reg (res) && TYPE_PRECISION (TREE_TYPE (res)) > precision)
> > >         precision = TYPE_PRECISION (TREE_TYPE (res));
> > >     }
> > > 
> > >   type = lang_hooks.types.type_for_size (precision, 1); // precision == 128 
> > >   ...
> > >  }
> > > 
> > > Does it seem that the precision should not determine the new type size, or that 
> > > the precision itself being 128 is strange?
> > Well, autopar seems to introduce this 128 bit type in the first place,
> > and I wonder why it does that.  And it definitely should avoid doing this.
> What happens is that autopar calls canonicalize_loop_ivs() when it is starting
> to change the loop.
> Here's a  part of the documentation of canonicalize_loop_ivs(): 
>  "  When the IV type precision has to be larger
>    than *NIT type precision, *NIT is converted to the larger type, the
>    conversion code is inserted before the loop, and *NIT is updated to
>    the new definition.  "  
> In this case of cactusADM, one of the loop's IVs indeed has a precision of 128,
> and therefore a conversion to a type of 128 bit is created.
> I checked the precision of the loop's IVs a few passes before autopar, and even
> when I disable autopar, and indeed there is an IV that has a type with 128
> precision.

I tried to build cactusADM on linux-x86 with autopar enabled, and I get 
segmentation fault due to the same reason.
It happens when either -m32c or -m64 is enabled.

/Develop/razya/gcc-cactus/bin/gcc -c -o PUGHReduce/ReductionNormInf.o
-DSPEC_CPU -DNDEBUG  -Iinclude -I../include -DCCODE  -O2
-ftree-parallelize-loops=4 -ffast-math       -DSPEC_CPU_LP64        
PUGHReduce/ReductionNormInf.c
PUGHReduce/ReductionNormInf.c: In function 'PUGH_ReductionNormInf':
PUGHReduce/ReductionNormInf.c:207:12: internal compiler error: Segmentation
fault
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
specmake: *** [PUGHReduce/ReductionNormInf.o] Error 1

The type is NULL at line 1218 in canonicalize_loop_ivs:

1214      type = lang_hooks.types.type_for_size (precision, 1);
1215    
1216      if (original_precision != precision)
1217        {
1218          *nit = fold_convert (type, *nit);
1219          *nit = force_gimple_operand (*nit, &stmts, true, NULL_TREE);
1220          if (stmts)
1221        gsi_insert_seq_on_edge_immediate (loop_preheader_edge (loop),
stmts);
1222        }

The size according to which the type is supposed to be created (line 1214) is
80.

[Bug tree-optimization/49471] ICE when -ftree-parallelize-loops is enabled together with -m32 on power7

[razya at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49471

razya at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2011.07.13 15:12:12
         AssignedTo|unassigned at gcc dot       |razya at gcc dot gnu.org
                   |gnu.org                     |
     Ever Confirmed|0                           |1

[Bug tree-optimization/49471] cactusADM/dealII build with autopar fails on x86, and fails on power7 when -m32 is enabled.

[razya at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49471

razya at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|cactusADM build with        |cactusADM/dealII build with
                   |autopar fails on x86, and   |autopar fails on x86, and
                   |fails on power7 when -m32   |fails on power7 when -m32
                   |is enabled.                 |is enabled.

--- Comment #6 from razya at gcc dot gnu.org 2011-07-25 13:31:50 UTC ---
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > (In reply to comment #2)
> > > > (In reply to comment #1)
> > > > > Why is
> > > > >   D.7313_5 = MEM[(struct  *).paral_data_param_1(D)].D.7288; /*  Number of loop
> > > > > iterations.  */
> > > > > of type __int128?  That looks bogus.
> > > > 
> > > > the size of 128 was determined according to the precision of the ivs in
> > > > canonicalize_loop_ivs:
> > > > 
> > > > canonicalize_loop_ivs (struct loop *loop, tree *nit, bool bump_in_latch)
> > > > {
> > > >   unsigned precision = TYPE_PRECISION (TREE_TYPE (*nit));   
> > > >   for (psi = gsi_start_phis (loop->header);         
> > > >        !gsi_end_p (psi); gsi_next (&psi))
> > > >     {
> > > >       gimple phi = gsi_stmt (psi);
> > > >       tree res = PHI_RESULT (phi);
> > > > 
> > > >       if (is_gimple_reg (res) && TYPE_PRECISION (TREE_TYPE (res)) > precision)
> > > >         precision = TYPE_PRECISION (TREE_TYPE (res));
> > > >     }
> > > > 
> > > >   type = lang_hooks.types.type_for_size (precision, 1); // precision == 128 
> > > >   ...
> > > >  }
> > > > 
> > > > Does it seem that the precision should not determine the new type size, or that 
> > > > the precision itself being 128 is strange?
> > > Well, autopar seems to introduce this 128 bit type in the first place,
> > > and I wonder why it does that.  And it definitely should avoid doing this.
> > What happens is that autopar calls canonicalize_loop_ivs() when it is starting
> > to change the loop.
> > Here's a  part of the documentation of canonicalize_loop_ivs(): 
> >  "  When the IV type precision has to be larger
> >    than *NIT type precision, *NIT is converted to the larger type, the
> >    conversion code is inserted before the loop, and *NIT is updated to
> >    the new definition.  "  
> > In this case of cactusADM, one of the loop's IVs indeed has a precision of 128,
> > and therefore a conversion to a type of 128 bit is created.
> > I checked the precision of the loop's IVs a few passes before autopar, and even
> > when I disable autopar, and indeed there is an IV that has a type with 128
> > precision.
> I tried to build cactusADM on linux-x86 with autopar enabled, and I get 
> segmentation fault due to the same reason.
> It happens when either -m32c or -m64 is enabled.
> /Develop/razya/gcc-cactus/bin/gcc -c -o PUGHReduce/ReductionNormInf.o
> -DSPEC_CPU -DNDEBUG  -Iinclude -I../include -DCCODE  -O2
> -ftree-parallelize-loops=4 -ffast-math       -DSPEC_CPU_LP64        
> PUGHReduce/ReductionNormInf.c
> PUGHReduce/ReductionNormInf.c: In function 'PUGH_ReductionNormInf':
> PUGHReduce/ReductionNormInf.c:207:12: internal compiler error: Segmentation
> fault
> Please submit a full bug report,
> with preprocessed source if appropriate.
> See <http://gcc.gnu.org/bugs.html> for instructions.
> specmake: *** [PUGHReduce/ReductionNormInf.o] Error 1
> The type is NULL at line 1218 in canonicalize_loop_ivs:
> 1214      type = lang_hooks.types.type_for_size (precision, 1);
> 1215    
> 1216      if (original_precision != precision)
> 1217        {
> 1218          *nit = fold_convert (type, *nit);
> 1219          *nit = force_gimple_operand (*nit, &stmts, true, NULL_TREE);
> 1220          if (stmts)
> 1221        gsi_insert_seq_on_edge_immediate (loop_preheader_edge (loop),
> stmts);
> 1222        }
> The size according to which the type is supposed to be created (line 1214) is
> 80.

dealII has the exact same behavior as cactusADM when autopar is enabled
on x86, or together with -m32 on powerpc.

[Bug tree-optimization/49471] cactusADM/dealII build with autopar fails on x86, and fails on power7 when -m32 is enabled.

[spop at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49471

--- Comment #7 from Sebastian Pop <spop at gcc dot gnu.org> 2011-07-27 16:53:15 UTC ---
Author: spop
Date: Wed Jul 27 16:53:09 2011
New Revision: 176838

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=176838
Log:
Fix PR49471: canonicalize_loop_ivs should not generate unsigned types.

2011-07-27  Sebastian Pop  <sebastian.pop@amd.com>

    PR tree-optimization/49471
    * tree-ssa-loop-manip.c (canonicalize_loop_ivs): Build an unsigned
    iv only when the largest type is unsigned.  Do not call
    lang_hooks.types.type_for_size.

    * testsuite/libgomp.graphite/force-parallel-1.c: Un-xfail.
    * testsuite/libgomp.graphite/force-parallel-2.c: Adjust pattern.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-ssa-loop-manip.c
    trunk/libgomp/ChangeLog
    trunk/libgomp/testsuite/libgomp.graphite/force-parallel-1.c
    trunk/libgomp/testsuite/libgomp.graphite/force-parallel-2.c

[Bug tree-optimization/49471] cactusADM/dealII build with autopar fails on x86, and fails on power7 when -m32 is enabled.

[spop at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49471

Sebastian Pop <spop at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
                 CC|                            |spop at gcc dot gnu.org
         Resolution|                            |FIXED

--- Comment #8 from Sebastian Pop <spop at gcc dot gnu.org> 2011-07-27 16:56:00 UTC ---
Fixed.