Re: [Patch, Fortran] PR fortran/35681: First part, fix ELEMENTAL dependency handling for MVBITS

[Paul Thomas <paul dot richard dot thomas at gmail dot com>]

Daniel,

Regtests fine on FC9/x86_i64.  This is OK for trunk.

Have you had any thoughts on parentheses expressions yet?

Thanks for the patch

Paul
> Hi,
>
> I've updated the patch described below to trunk of now (including the 
> trivial conflicts merge with Mikael's recent check-in) and run a new 
> regtest, no regressions on GNU/Linux-x86-32.
>
> Cheers,
> Daniel
>
> Daniel Kraft wrote:
> > working on PR fortran/35681, I've got some rather big patch now 
> > handling part of the problem.  What it exactly does:
> >
> > 1) Some tab-indentation formatting fixes as I came along, sorry for 
> > those.  I hope it is ok so.
> >
> > 2) When resolving a MVBITS intrinsic call, the code->resolved_sym 
> > gets a dummy formal argument list with the correct INTENTs specified; 
> > this is needed later for gfc_conv_elemental_dependencies.
> >
> > 3) gfc_code got a new member "resolved_isym" that tracks calls to 
> > intrinsic procedures, so we can later check if some call is to 
> > intrinsic MVBITS.  This got a little ugly and would be probably nicer 
> > to union it (and possibly "resolved_sym", too) with actual, but that 
> > would probably introduce a lot of changes to existing code pieces.
> >
> > 4) gfc_trans_allocate_array_storage (or what it is called) got a new 
> > argument `initial' that allows to initialize the created storage from 
> > some other array (this is done using a combination of internal_pack 
> > and memcpy if it was already packed, I hope I got this all right).  
> > This is used for gfc_trans_create_temp_array to allow initializing 
> > the new temporary.  Here is (probably) most of the "critical" changes.
> >
> > 5) For calls to intrinsic MVBITS, I enabled dependency checking using 
> > gfc_conv_elemental_dependencies and made this routine aware of 
> > INTENT(INOUT) arguments that use the new initialization feature to 
> > copy over the initial content of the mirrored array to the created 
> > temporary.
> >
> > 6) I could not find a test to verify this (not even one that uses 
> > gfc_conv_elemental_dependencies) in a quick trial, but I believe the 
> > handling of the temporary there was wrong, in that it was free'd (if 
> > allocated on the heap) *before* it was used with internal_unpack, 
> > because gfc_trans_create_temp_array added the temporary clean-up code 
> > to se->post and the unpack-call was added to se->post later.  In my 
> > opinion, this is some rather general problem with how post-commands 
> > are usually added to other post blocks; shouldn't they be added to 
> > the top usually rather than to the bottom, to get some sort of 
> > "nested" scope with inner most pairs of pre/post?  Well, for now I 
> > changed this behaviour inside gfc_conv_elemental_dependencies, which 
> > corrected problems I got with MVBITS tests.
> >
> > This enabled the (valid) tests in the PR to run, but only with 
> > modifying them slightly by removing the parentheses around the first 
> > argument (so it is not an expression; that will be part 2 of this 
> > fix).  As I understand it, this is valid in case of MVBITS but not 
> > for any other ELEMENTAL subroutine, right?  This is why I added the 
> > check for whether some call is to MVBITS.  I guess the rationale why 
> > the compiler is not required to create temporaries for all such 
> > ELEMENTAL calls (and they are invalid instead) is performance?  
> > gfortran could handle those calls well in addition to only MVBITS 
> > calls simply if I take this conditional check out, but then we might 
> > generate temporaries for cases where the user knows no one is needed 
> > and the code is valid but the compiler can't figure it out.
> >
> > I hope I got this one at least somewhat clear...  What do you think 
> > about it?  Currently regression-testing on GNU/Linux-x86-32, but I 
> > don't expect any (a very similar patch worked fine before).
> >
> > Cheers,
> > Daniel