public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "burnus at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug fortran/107424] [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops
Date: Fri, 13 Jan 2023 10:46:03 +0000	[thread overview]
Message-ID: <bug-107424-4-nPjWaRXxUE@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-107424-4@http.gcc.gnu.org/bugzilla/>

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107424

--- Comment #4 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Created attachment 54265
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54265&action=edit
WIP patch (with one WIP testcase)

This patch should address the issues of this PR reported so far.

But there two issues: (A) I am not sure whether the implicit 'lastprivate'
should be added for SIMD or not. (B) there is an odd issue with lastprivate
after gimplify, before it looks fine.

(A) 'lastprivate' or not (refers to the patch itself):
The SIMD/!simple change is because of:
  If there is no unit stride, 'count' is created (+ 'lastprivate(count)').
  For collapse == 1, 'linear' is added and later due to the 'dovar_found = 3',
    OMP_CLAUSE_LINEAR_STMT (c) = ...
  For collapse > 1, nothing was added – such that the code trying to add
  a ..._STMT failed to find a clause + and the 'c != NULL' assert then failed.

  The current patch adds 'lastprivate' in this case – an alternative would be
  to move the 'dovar_found = 3' into the 'if'.
  (The lastprivate(count) would still be added.)

  It is not quite clear to me what OpenMP requires - lastprivate or nothing
  (TODO: check).


(B) The following code goes wrong; I converted it as closely as possible to C
and the C program works fine.

  integer :: n,m,p, i,j,k
  n = 11; m = 23; p = 27
  !$omp simd collapse(3) lastprivate(k)
  do i = 1, n
    do j = 1, m
      do k = j - 41, p
        if (k < 1 - 41 .or. k > p) error stop
      end do
    end do
  end do
  end

First, without the patch, the 'j - 41' is evaluated before the loop, which is
obviously wrong:
    D.4268 = j + -41;
    #pragma omp simd lastprivate(k) collapse(3)
      ...
        for (k = D.4268; k <= p; k = k + 1)

But with the patch, it seems to be fine:

  #pragma omp simd lastprivate(j) lastprivate(i) lastprivate(k) collapse(3)
  for (i = 1; i <= n; i = i + 1)
    for (j = 1; j <= m; j = j + 1)
      for (k = j + -41; k <= p; k = k + 1)
            if (k < -40 || k > p)

However, the gimple dump has the following - note the 'k = D.4287' where the
RHS is uninitialized!

      n = 11;
      m = 23;
      p = 27;
      #pragma omp simd lastprivate(j) lastprivate(i) lastprivate(k) collapse(3)
private(k.9) private(j.6) private(i.3) private(dt_parm.0)
      for (i.3 = 1; i.3 <= n; i.3 = i.3 + 1)
        for (j.6 = 1; j.6 <= m; j.6 = j.6 + 1)
          for (k.9 = D.4287; k.9 <= p; k.9 = k.9 + 1)


BTW: The attached patch contains a testcase from which (B) has been extracted
as reduced fail.

TODO: Besides fixing issue (B) and checking (A), some more testcases are needed
+ fixing issues popping up when doing so.

(PS: Currently, the start/end/step expressions are put into the loop even if
they do not depend on another loop variable - this could be optimized, but my
feeling is that this is not worthwhile.)

  parent reply	other threads:[~2023-01-13 10:46 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-26 18:37 [Bug fortran/107424] New: [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 gscfq@t-online.de
2022-10-27  8:50 ` [Bug fortran/107424] " marxin at gcc dot gnu.org
2022-10-28 11:35 ` rguenth at gcc dot gnu.org
2023-01-11 15:30 ` [Bug fortran/107424] [13 Regression] ICE in gfc_trans_omp_do, at fortran/trans-openmp.cc:5397 - and wrong code - with non-rectangular loops burnus at gcc dot gnu.org
2023-01-11 15:59 ` burnus at gcc dot gnu.org
2023-01-13 10:46 ` burnus at gcc dot gnu.org [this message]
2023-01-13 10:53 ` burnus at gcc dot gnu.org
2023-01-13 11:07 ` jakub at gcc dot gnu.org
2023-01-13 11:46 ` burnus at gcc dot gnu.org
2023-01-13 11:51 ` jakub at gcc dot gnu.org
2023-01-18 22:24 ` burnus at gcc dot gnu.org
2023-01-25 14:50 ` burnus at gcc dot gnu.org
2023-02-09 14:51 ` cvs-commit at gcc dot gnu.org
2023-02-09 14:54 ` burnus at gcc dot gnu.org
2023-04-26  6:56 ` [Bug fortran/107424] [13/14 " rguenth at gcc dot gnu.org
2023-07-17 11:15 ` burnus at gcc dot gnu.org
2023-07-19  8:19 ` cvs-commit at gcc dot gnu.org
2023-07-19  8:27 ` burnus at gcc dot gnu.org

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bug-107424-4-nPjWaRXxUE@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).