[patch] PR 37950

[patch] PR 37950

[Zdenek Dvorak]

Hi,

consider a loop

for (i = 0; i < n; i++)
  ... = arr[i];

where memory_partition (arr) = MPT.1.  When autoparallelization moves
the statement to the new function, we rescan its arguments, however
a reference to MPT.1 remains inside gimple_loaded_syms of the statement.
This leads to ice during the compilation of the new function.

The patch below ensures that when we are scanning the operands of the
statement in the context of the new function, we consider
memory_partition (arr) to be NULL as expected.  Bootstrapped & regtested
on i686.

Zdenek

	* tree-flow-inline.h (memory_partition): Return NULL when aliases were
	not computed for the current function.

Index: tree-flow-inline.h
===================================================================
*** tree-flow-inline.h	(revision 141718)
--- tree-flow-inline.h	(working copy)
*************** memory_partition (tree sym)
*** 644,649 ****
--- 644,655 ----
      return sym;
  
    gcc_assert (!is_gimple_reg (sym));
+   /* Autoparallelization moves statements from the original function (which has
+      aliases computed) to the new one (which does not).  When rebuilding
+      operands for the statement in the new function, we do not want to
+      record the memory partition tags of the original function.  */
+   if (!gimple_aliases_computed_p (cfun))
+     return NULL_TREE;
    tag = get_var_ann (sym)->mpt;
  
  #if defined ENABLE_CHECKING

Re: [patch] PR 37950

[Diego Novillo]

2008/11/11 Zdenek Dvorak <rakdver@kam.mff.cuni.cz>:

>
>        * tree-flow-inline.h (memory_partition): Return NULL when aliases were
>        not computed for the current function.

It's better to clear out gimple_loaded_syms() from the moved
statements.  In fact, build_ssa_operands should probably clear
loaded/stored syms (I thought it did already).


Diego.

Re: [patch] PR 37950

[Zdenek Dvorak]

Hi,

> >        * tree-flow-inline.h (memory_partition): Return NULL when aliases were
> >        not computed for the current function.
> 
> It's better to clear out gimple_loaded_syms() from the moved
> statements.  In fact, build_ssa_operands should probably clear
> loaded/stored syms (I thought it did already).

unfortunately, that is not possible -- that would give you empty
gimple_loaded_syms, breaking alias analysis (unless you are willing
to rescan all statement operands at the beginning of alias analysis;
I have considered that, but since rescanning the operands is rather
expensive, I think it is better to get the operands right the first
time),

Zdenek

Re: [patch] PR 37950

[Richard Guenther]

On Wed, Nov 12, 2008 at 11:17 AM, Zdenek Dvorak <rakdver@kam.mff.cuni.cz> wrote:
> Hi,
>
>> >        * tree-flow-inline.h (memory_partition): Return NULL when aliases were
>> >        not computed for the current function.
>>
>> It's better to clear out gimple_loaded_syms() from the moved
>> statements.  In fact, build_ssa_operands should probably clear
>> loaded/stored syms (I thought it did already).
>
> unfortunately, that is not possible -- that would give you empty
> gimple_loaded_syms, breaking alias analysis (unless you are willing
> to rescan all statement operands at the beginning of alias analysis;
> I have considered that, but since rescanning the operands is rather
> expensive, I think it is better to get the operands right the first
> time),

For fixing PR38051 I will have to remove using the loaded/stored syms
from alias analysis.  So it may work to clean them after that.

Richard.

> Zdenek
>

Re: [patch] PR 37950

[Zdenek Dvorak]

Hi,

> >> >        * tree-flow-inline.h (memory_partition): Return NULL when aliases were
> >> >        not computed for the current function.
> >>
> >> It's better to clear out gimple_loaded_syms() from the moved
> >> statements.  In fact, build_ssa_operands should probably clear
> >> loaded/stored syms (I thought it did already).
> >
> > unfortunately, that is not possible -- that would give you empty
> > gimple_loaded_syms, breaking alias analysis (unless you are willing
> > to rescan all statement operands at the beginning of alias analysis;
> > I have considered that, but since rescanning the operands is rather
> > expensive, I think it is better to get the operands right the first
> > time),
> 
> For fixing PR38051 I will have to remove using the loaded/stored syms
> from alias analysis.  So it may work to clean them after that.

yes, this would work as well, I think.  I am a bit worried about leaving
the statements with the incorrect (or empty) loaded/stored_syms, though;
the passes that are before pass_build_alias do not seem to use this
information, but that could change,

Zdenek

Re: [patch] PR 37950

[Richard Guenther]

On Wed, Nov 12, 2008 at 2:30 PM, Zdenek Dvorak <rakdver@kam.mff.cuni.cz> wrote:
> Hi,
>
>> >> >        * tree-flow-inline.h (memory_partition): Return NULL when aliases were
>> >> >        not computed for the current function.
>> >>
>> >> It's better to clear out gimple_loaded_syms() from the moved
>> >> statements.  In fact, build_ssa_operands should probably clear
>> >> loaded/stored syms (I thought it did already).
>> >
>> > unfortunately, that is not possible -- that would give you empty
>> > gimple_loaded_syms, breaking alias analysis (unless you are willing
>> > to rescan all statement operands at the beginning of alias analysis;
>> > I have considered that, but since rescanning the operands is rather
>> > expensive, I think it is better to get the operands right the first
>> > time),
>>
>> For fixing PR38051 I will have to remove using the loaded/stored syms
>> from alias analysis.  So it may work to clean them after that.
>
> yes, this would work as well, I think.  I am a bit worried about leaving
> the statements with the incorrect (or empty) loaded/stored_syms, though;
> the passes that are before pass_build_alias do not seem to use this
> information, but that could change,

The patch I applied for PR38051 doesn't fully get rid of stored/loaded_syms,
so I think your original patch is ok.

Thanks,
Richard.

> Zdenek
>