[flang][OpenMP] ordered is ignored in target offload, giving wrong results

[sbryngelson]

The Fortran equivalent of an upstream offload runtime test fails on the same GPU where the C
version passes. ordered inside a target region does not order anything in flang.

offload/test/offloading/schedule.c is shipped upstream and verifies that an ordered region on a
target construct executes in iteration order. It passes on gfx90a:

$ clang -fopenmp --offload-arch=gfx90a -O2 offload/test/offloading/schedule.c -o schedc && ./schedc
test no order OK
test ordered OK

A line-for-line Fortran port of the ordered_example part of that test fails:

program p
  use omp_lib
  implicit none
  integer, parameter :: lb = 0, ub = 100, stride = 1, nteams = 8
  integer, parameter :: sz = (ub - lb) / stride
  real(8) :: output(sz)
  integer :: i, j, jj, bad
  output = 0.0d0
  !$omp target teams map(from:output) num_teams(nteams) thread_limit(128)
  !$omp parallel do ordered schedule(dynamic)
  do i = lb, ub - 1, stride
     !$omp ordered
     output((i - lb)/stride + 1) = omp_get_wtime()
     !$omp end ordered
  end do
  !$omp end target teams
  bad = 0
  do j = 1, sz
     do jj = j + 1, sz
        if (output(j) > output(jj)) bad = bad + 1
     end do
  end do
  if (bad == 0) then
     print *, "test ordered OK"
  else
     print '(A,I6,A)', "  Fail to schedule in order: ", bad, " inversions"
  end if
end program

build	result
clang, the C test, gfx90a	test ordered OK
flang, the port, gfx90a	Fail to schedule in order: 2048 / 1024 / 1280 inversions
flang, the port, host, OMP_NUM_THREADS=8	test ordered OK
amdflang AFAR 23.2.1, gfx90a	fails likewise

The inversion count varies between runs, as expected for a race. A smaller deterministic variant,
recording the order in which iterations enter the region, shows every iteration reading the same
counter, so the region provides no ordering or mutual exclusion at all.

Cause

ordered needs a dispatch schedule so the runtime can enforce ordering through
__kmpc_dispatch_fini. clang's device IR for the C test has __kmpc_dispatch_init_4,
__kmpc_dispatch_next_4 and __kmpc_dispatch_fini_4 alongside __kmpc_ordered. flang's has
__kmpc_for_static_loop_4u with __kmpc_ordered, so there is nothing for __kmpc_ordered to
order against.

OpenMPIRBuilder::applyWorkshareLoop discards its schedule-related arguments on the device path
(llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:6497):

if (Config.isTargetDevice())
  return applyWorkshareLoopTarget(DL, CLI, AllocaIP, LoopType, NoLoop);

HasOrderedClause is dropped there, along with SchedKind, ChunkSize, HasDistSchedule and
DistScheduleChunkSize. The schedule(dynamic) in this same test is therefore also ignored; I will
file that separately since the severity is different.

Either route ordered through the dispatch path on device as clang does, or reject the combination
with a diagnostic until that works.

Notes

I have not bisected, so I cannot say whether this is a regression or has always been the case.

[sbryngelson]

@kparzysz @tblah flagging this one since you are both on the flang OpenMP side. The reproducer is a Fortran port of offload/test/offloading/schedule.c, which passes in C on the same GPU.

[sbryngelson]

Correction: the reproducer I posted is flaky, please use this one instead.

The port of offload/test/offloading/schedule.c compares omp_get_wtime() values, so the threads
can happen to come out in order. On unfixed flang it is 16 pass / 4 fail over 20 runs on
gfx90a. I measured three failing runs and posted it as if it failed consistently, which was wrong
and would have wasted your time.

This one records the order in which iterations enter the ordered region, so it does not depend on
timing. It fails 20 of 20 on the same build:

program p
  implicit none
  integer, parameter :: n = 64
  integer :: seq(n), pos, i, bad
  pos = 0
  seq = -1
  !$omp target parallel do ordered map(tofrom:seq,pos)
  do i = 1, n
     !$omp ordered
     pos = pos + 1
     seq(pos) = i
     !$omp end ordered
  end do
  bad = 0
  do i = 1, n
     if (seq(i) /= i) bad = bad + 1
  end do
  if (bad == 0) then
     print *, "PASS ordered preserved"
  else
     write(*,'(A,I4,A)') "  FAIL: ", bad, " iterations out of order"
     write(*,'(A,16I4)') "  first 16: ", seq(1:16)
  end if
end program

$ flang -fopenmp --offload-arch=gfx90a -O3 ord.f90 -o ord && ./ord
  FAIL:   64 iterations out of order
  first 16:   61  -1  -1  -1  -1  -1  -1  -1  -1  -1  -1  -1  -1  -1  -1  -1

Every thread reads the same pos, so only one write survives. Host and clang-on-gfx90a both print
PASS ordered preserved, as before.

The C test is still worth citing as evidence that the pattern is supported and exercised upstream
for offload, but it is not a reliable reproducer for this bug.