Bug 33749 - Wrong evaluation of expressions in lhs of assignment statements
Summary: Wrong evaluation of expressions in lhs of assignment statements
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.3.0
: P3 normal
Target Milestone: ---
Assignee: Paul Thomas
URL:
Keywords: wrong-code
Depends on:
Blocks: 32834
  Show dependency treegraph
 
Reported: 2007-10-12 13:36 UTC by Paul Thomas
Modified: 2007-10-30 22:20 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2007-10-18 04:26:21


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Thomas 2007-10-12 13:36:24 UTC
This is a similar problem to PR33686.

$ cat test.f90
  integer :: p(4) = (/2,4,1,3/)
  p(p) = (/(i, i = 1, 4)/)
  print *, p
end

$ ./a
           3           1           4           3

7.5.1.5 Interpretation of intrinsic assignments
Execution of an intrinsic assignment causes, in effect, the evaluation of the expression expressions within variable (7.1.7), the possible conversion of expr to the type and type of variable (Table 7.10), and the definition of variable with the resulting value. The execution assignment shall have the same effect as if the evaluation of all operations in expr occurred before any portion of variable is defined by the assignment. The evaluation within variable shall neither affect nor be affected by the evaluation of expr. No value variable if variable is of type character and zero length, or is an array of size zero.

....snip....

The processor may perform the element-by-element assignment in any order.

I take all this to mean that expressions within variable must be evaluated before the assignment, so that the correct result should be

$ ./a
           3           1           4           2

Paul
Comment 1 Tobias Burnus 2007-10-12 15:18:17 UTC
I want to add that I cannot reproduce it on x86-64/Linux with -m64 -- only with -m32. Other than that, I agree that this is a bug.
Comment 2 Dominique d'Humieres 2007-10-12 15:23:23 UTC
The same on PPC Darwin: pr33749 with -m64 gives the expected result, but pr33686 gives the same result for 32 and 64 bit modes.
Comment 3 Paul Thomas 2007-10-12 19:09:30 UTC
(In reply to comment #2)
> The same on PPC Darwin: pr33749 with -m64 gives the expected result, but
> pr33686 gives the same result for 32 and 64 bit modes.
> 
Tobias and Dominique,

You're right.  The assignment even produces the temporary for the lhs index that it should.  Now why on earth does this happen in 64bit mode an not in 32bit??

Try it; first you see it and then you don't....

Still worse, I failed to find any code that would make this possible!! OK I've got airport lounges and hotel rooms next week; I'll investigate then.

Cheers

Paul
Comment 4 Francois-Xavier Coudert 2007-10-17 12:36:25 UTC
(In reply to comment #3)
> You're right.  The assignment even produces the temporary for the lhs index
> that it should.  Now why on earth does this happen in 64bit mode an not in
> 32bit??

Sometimes, the difference between 32 and 64 bit is that gfortran generates conversions for subscripts from int4 to int8 in the 64-bit case, while it uses the int4 directly in the 32 bit case; an expression can then be an EXPR_FUNCTION in one case (__convert_i4_i8, or something like that) and an EXPR_CONST or EXPR_ARRAY in the other case; this difference then makes us take different code paths. I've seen it happen for a recent PR, though I can't remember which.
Comment 5 Paul Thomas 2007-10-18 04:26:20 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > You're right.  The assignment even produces the temporary for the lhs index
> > that it should.  Now why on earth does this happen in 64bit mode an not in
> > 32bit??
> 
> Sometimes, the difference between 32 and 64 bit is that gfortran generates
> conversions for subscripts from int4 to int8 in the 64-bit case, while it uses
> the int4 directly in the 32 bit case; an expression can then be an
> EXPR_FUNCTION in one case (__convert_i4_i8, or something like that) and an
> EXPR_CONST or EXPR_ARRAY in the other case; this difference then makes us take
> different code paths. I've seen it happen for a recent PR, though I can't
> remember which.
> 

FX,

Yes, I just beat you to it http://gcc.gnu.org/ml/fortran/2007-10/msg00207.html :)

I'll try to post the patch tomorrow.

Cheers

Paul
Comment 6 Paul Thomas 2007-10-21 18:10:19 UTC
Subject: Bug 33749

Author: pault
Date: Sun Oct 21 18:10:00 2007
New Revision: 129539

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=129539
Log:
2007-10-21  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/33749
	* resolve.c (resolve_ordinary_assign): New function that takes
	the code to resolve an assignment from resolve_code. In
	addition, it makes a temporary of any vector index, on the
	lhs, using gfc_get_parentheses.
	(resolve_code): On EXEC_ASSIGN call the new function.

2007-10-21  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/33749
	* gfortran.dg/assign_9.f90: New test.


Added:
    trunk/gcc/testsuite/gfortran.dg/assign_9.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/resolve.c
    trunk/gcc/testsuite/ChangeLog

Comment 7 Tobias Burnus 2007-10-23 08:20:23 UTC
(In reply to comment #6)
> URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=129539
> Log:
> 2007-10-21  Paul Thomas  <pault@gcc.gnu.org>
> 
>         PR fortran/33749

Paul, can this bug be closed as FIXED or is something left to be fixed?
Or should be backported to 4.2?
Comment 8 patchapp@dberlin.org 2007-10-30 22:15:18 UTC
Subject: Bug number PR33749

A patch for this bug has been added to the patch tracker.
The mailing list url for the patch is http://gcc.gnu.org/ml/gcc-patches/2007-10/msg01237.html
Comment 9 Francois-Xavier Coudert 2007-10-30 22:20:38 UTC
(In reply to comment #8)
> Subject: Bug number PR33749
> 
> A patch for this bug has been added to the patch tracker.
> The mailing list url for the patch is
> http://gcc.gnu.org/ml/gcc-patches/2007-10/msg01237.html

Hum, patchapp sure isn't very up-to-date with his work :)
The patch was approved, and I think the PR can be closed as FIXED.