Re: Possible patch for fortran/55086

Re: Possible patch for fortran/55086

[Dominique d'Humières]

> The problem was that forall could handle substring expressions,
> but not if it had to create a temporary array.  I did my best
> to fix this.  I left a couple of TODO comments in the code.

Well, I don’t understand why the test needs a temporary for

  forall (i1 = nl1:nu1) o(i1:i1) = i(i1:i1)   ! <<<< ICE

If I am not mistaken this is equivalent to

  o(nl1:nu1) = i(nl1:nu1)

for which gfortran does not generate any temporary while it does for
  forall (i1 = nl1:nu1) o(i1:i1) = o(nu1+1-i1:nu1+1-i1) 
where it is needed (looking at the dumps when compiling with -fdump-tree-original).

Note that your patch generates a temporary which is not needed.

At this point I have two questions:
(1) why does gfortran want to create a temporary?
(2) why does it fails?

> Part of the issue is that forall doesn't usually create
> a temporary, so that code doesn't get as much test coverage
> as it probably should.

In an ideal world, the compiler should create temporaries for assignment only when needed. AFAICR the coverage is rather good for usual assignments, I’ld be surprised if this were the case for FORALL.

>  I added an option (-fforce-forall-temp)
> to force forall to use a temporary, and I set that option
> in a couple of the test files.

I am not really fond of this option. At best, it could only be a work around missing temporaries. What would be more helpful would be to report temporary creations when compiling with -Warray-temporaries or -fcheck=array-temps.

Thanks for working on the issue.

Dominique

> I would not be surprised if there were a better way to do this.
>
> When I tweak gfc_trans_forall_1() to force usage of a temporary
> for all assignments, all tests pass with the exception
> of forall_8.f90, and the whole point of that test is making
> sure that this particular forall *doesn't* use a temporary.
>  
> Louis Krupp

Re: Possible patch for fortran/55086

[Louis Krupp]

Dominique,

(1) I'm not sure why the 'forall' gets a temporary.
(2) As far as I can tell, we get an ICE because the code that generates 'forall' temporaries doesn't work well with substrings.  It's hard to test all possible combinations, and this one had been missed.

While the original program in the bug report might not have really needed a temporary, other programs will, and unless there's a way to avoid using temporaries for the combination of 'forall' and substrings, the code that generates temporaries will have to be fixed.  I'm suggesting '-fforce-forall-temp' as a way to make sure this code gets tested more thoroughly;  it's for one or two specific test files only, and not for general use.  I'd rather test code that will never be executed than leave code untested if there's even a small chance that it will be executed some day.

'gfortran --help -v' would list the option as '-fforce-forall-temp: Force creation of temporary to debug forall code'.  I can hide it completely if necessary.

As far as I know, my suggested patch doesn't generate any new temporaries unless -fforce-forall-temp is set.  I could be wrong.  It wouldn't be the first time.  I can take another look at that.

Louis



 ---- On Sun, 06 Nov 2016 08:26:24 -0800 Dominique d'Humières <dominiq@lps.ens.fr> wrote ---- 
 > > The problem was that forall could handle substring expressions, 
 > > but not if it had to create a temporary array.  I did my best 
 > > to fix this.  I left a couple of TODO comments in the code. 
 >  
 > Well, I don’t understand why the test needs a temporary for 
 >  
 >   forall (i1 = nl1:nu1) o(i1:i1) = i(i1:i1)   ! <<<< ICE 
 >  
 > If I am not mistaken this is equivalent to 
 >  
 >   o(nl1:nu1) = i(nl1:nu1) 
 >  
 > for which gfortran does not generate any temporary while it does for 
 >   forall (i1 = nl1:nu1) o(i1:i1) = o(nu1+1-i1:nu1+1-i1)  
 > where it is needed (looking at the dumps when compiling with -fdump-tree-original). 
 >  
 > Note that your patch generates a temporary which is not needed. 
 >  
 > At this point I have two questions: 
 > (1) why does gfortran want to create a temporary? 
 > (2) why does it fails? 
 >  
 > > Part of the issue is that forall doesn't usually create 
 > > a temporary, so that code doesn't get as much test coverage 
 > > as it probably should. 
 >  
 > In an ideal world, the compiler should create temporaries for assignment only when needed. AFAICR the coverage is rather good for usual assignments, I’ld be surprised if this were the case for FORALL. 
 >  
 > >  I added an option (-fforce-forall-temp) 
 > > to force forall to use a temporary, and I set that option 
 > > in a couple of the test files. 
 >  
 > I am not really fond of this option. At best, it could only be a work around missing temporaries. What would be more helpful would be to report temporary creations when compiling with -Warray-temporaries or -fcheck=array-temps. 
 >  
 > Thanks for working on the issue. 
 >  
 > Dominique 
 >  
 > > I would not be surprised if there were a better way to do this. 
 > > 
 > > When I tweak gfc_trans_forall_1() to force usage of a temporary 
 > > for all assignments, all tests pass with the exception 
 > > of forall_8.f90, and the whole point of that test is making 
 > > sure that this particular forall *doesn't* use a temporary. 
 > >   
 > > Louis Krupp 
 >  
 >  
 >