This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH, RTL]: Fix PR 63483, Scheduler performs Invalid move of aliased memory reference


Hello!

This message revives an old thread [1], where the miscompilation of gfortran
on alpha was found that that resulted in:

FAIL: gfortran.dg/assumed_rank_3.f90:15.20:

    print *, ubound(x,dim=3)  ! << wrong dim
                    1
Error: Assumed-rank variable x at (1) may only be used as actual argument

The problem was in the miscompilation of resolve_actual_arglist from
resolve.c.  This function initializes two nearby global bool variables
with the following sequence:

  actual_arg = true;
  first_actual_arg = true;

but due to the miscompilation, the actual_arg was never set to true.
This happened due to the way stores to QImode and HImode locations are
implemented on non-BWX targets. The sequence reads full word, does its
magic to the part and stores the full word with changed part back to
the memory. However - the scheduler mixed two sequences, violating the
atomicity of RMW sequence.

As demostrated in the great detail in the PR [2], the problem is in
early exit  for MEM_READOLNY_P in true_dependence_1 in alias.c. This
early exit declares all MEM_READONLY_P references as non-aliasing,
which is not true when possibly aliasing references with alignment
ANDs are involved.

Proposed patch prevents MEM_READONLY_P memory references to be moved
over possibly aliased memory (with alignment ANDs). The patch prevents
early exit for MEM_READONLY_P references when alignment ANDs are
involved. The aliasing is then determined later in the function. In
effect, it changes early exit to:

-  if (MEM_READONLY_P (x))
-    return 0;
+  if (MEM_READONLY_P (x)
+      && GET_CODE (x_addr) != AND
+      && GET_CODE (mem_addr) != AND)
+    return 0;

The comment also mentions "... We don't expect to find read-only set
on MEM, but stupid user tricks can produce them, so don't die.". We
certainly don't die anymore, as confirmed by a native alpha-linux-gnu
(please note - not alphaev6!) bootstrap and regression test.

2014-10-08  Uros Bizjak  <ubizjak@gmail.com>

    * alias.c (true_dependence_1): Do not exit early for MEM_READONLY_P
    references when alignment ANDs are involved.

The patch was bootstrapped and regression tested on alpha-linux-gnu.

OK for mainline and release branches?

Please note that the patch by itself is not enough to fix the original
problem with gfortran miscompilation. Another problem in this area is
summarised in PR 63475 [3], where postreload CSE propagates aliased
memory operand.

[1] https://gcc.gnu.org/ml/gcc-patches/2014-06/msg02251.html
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63483
[3] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63475

Uros.

Attachment: alias.diff.txt
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]