llvm / llvm/llvm-project

[flang][OpenMP] Support ompx_attribute clause (RFC on Fortran spelling)

Open
#211,133 0 comments 0 reactions 0 assignees View on GitHub
flang:openmp
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

clang accepts `ompx_attribute` on OpenMP target constructs and lowers it to LLVM function attributes. flang rejects it at parse time, so Fortran OpenMP offload cannot set per-kernel launch bounds or occupancy hints at all.

```
$ flang -fopenmp --offload-arch=gfx90a -O3 ompx_attribute.f90 -o /dev/null
error: Could not parse ompx_attribute.f90
ompx_attribute.f90:20:85: error: expected '=>'
ompx_attribute.f90:20:45: in the context: pointer assignment statement
```

clang lowers the equivalent for real: device IR gains `"amdgpu-waves-per-eu"="4,4"` with the clause and lacks it without.

ompx_attribute.f90

```fortran
! Reproducer: ompx_attribute is accepted by clang for C/C++ but is a parse
! error in flang, leaving Fortran OpenMP offload with no way to set per-kernel
! occupancy or launch-bound hints.
!
! amdflang -fopenmp --offload-arch=gfx90a -O3 ompx_attribute.f90 -o /dev/null
!
! Observed (ROCm 7.2.0, AMD flang 22.0.0git):
! error: Could not parse ompx_attribute.f90
! error: expected end of line
! !$omp target teams distribute parallel do ompx_attribute(...)
! ^
! Expected: accepted, lowering to the "amdgpu-waves-per-eu" function attribute,
! matching clang's behaviour on ompx_attribute.c.

program p
implicit none
real(8) :: a(1000)
integer :: i
a = 1.0d0
!$omp target teams distribute parallel do ompx_attribute(amdgpu_waves_per_eu(4,4))
do i = 1, 1000
a(i) = a(i)*2.0d0
end do
print *, sum(a)
end program p
```

Verified at 119b31fd3, there are two gaps rather than one. `OMPC_OMPX_Attribute` (OMP.td:421) has a `clangClass` but no `flangClass`, so tablegen emits `EMPTY_CLASS(OmpxAttribute)`, `ClauseT.h:1032` declares `OmpxAttributeT` as `EmptyTrait`, `Clauses.cpp:286` is `MAKE_EMPTY_CLASS`, and there is no parser rule. Separately, the clause is not in the allowed-clause list of any Fortran-spelled directive: mapping all 34 `VersionedClause` sites shows every C-spelled `...ParallelFor...` form present and no `...ParallelDo...` form. `OMP_TargetTeamsDistributeParallelDo` (OMP.td:2529, `let languages = [L_Fortran]`) lacks it, so a parser rule alone would parse and then fail the semantic check. The language-neutral directives do allow it. Note `ompx_dyn_cgroup_mem` has a `flangClass`, a `make()` and a semantic `Enter()` but no parser rule either, so it is a shape reference rather than working prior art.

Filing this as an RFC rather than a PR because the Fortran spelling needs a decision. clang parses the clause body with `ParseAttributes(PAKM_GNU | PAKM_CXX11, ...)`, accepting `ompx_attribute(__attribute__((amdgpu_waves_per_eu(4,4))))` and `ompx_attribute([[clang::amdgpu_waves_per_eu(4,4)]])`, and explicitly rejecting a bare attribute name (`clang/test/OpenMP/ompx_attributes_messages.cpp`: `ompx_attribute(baz)` gives `expected ')'`). Fortran has neither spelling, so any Fortran syntax is a divergent extension. Candidates: a bare name with an integer list, `ompx_attribute(amdgpu_waves_per_eu(4,4))`, which is minimal but is the form clang rejects; accepting the clang forms verbatim for source portability; or a modifier style closer to modern clause grammar, `ompx_attribute(amdgpu_waves_per_eu: 4, 4)`.

A working proof-of-concept for the first option exists (881 lines, 18 files, applies clean to 119b31fd3), covering parser, semantics, `ClauseT.h`, an `omp.target` attribute, and lowering that mirrors clang's split: `amdgpu_waves_per_eu` onto the outlined function, `launch_bounds` and `amdgpu_flat_work_group_size` through `TargetKernelDefaultAttrs` so `writeThreadBoundsForKernel` / `writeTeamsForKernel` apply them. Output matches clang on the C equivalent for all three attributes, and tracks the clause value: `(4,4)` gives `"amdgpu-waves-per-eu"="4,4"`, `(8,8)` gives `"8,8"`. Four tests, each confirmed to fail without the patch; flang lit 4791 to 4794 discovered with 0 failures before and after, MLIR OpenMP and Target suites 0 failures.

I will post the patch once the spelling is settled. Two notes for whoever takes it: the attribute name must be a `std::string` rather than a `parser::Name` or name resolution fails, and `OpenMP.cpp` carries a second allow-list (a `TODO(... "clause is not implemented yet")` guard for block constructs) that must also be updated or `!$omp target teams ompx_attribute(...)` aborts the compiler.

There is no workaround. `!DIR$ AMDGPU_WAVES_PER_EU n` gives `warning: Unrecognized compiler directive was ignored`, `thread_limit(64..1024)` has no effect on register budget or occupancy, and no `-mllvm` or driver option exists.

Prior art: #99927, #195665, #195203 on the clang side, plus user reports #64815 and #64816. On the flang side #100370 already added the `applyClause(OmpxAttributeT...)` overload in `ConstructDecompositionT.h`, so construct decomposition is done.

This affects performance-critical applications on large AMD GPU supercomputers, including [MFC](https://github.com/MFlowCode/MFC).

All numbers above come from the validated reproducers included with this report and are independently reproducible; they stand on their own.

This was found and root-caused with the assistance of AI tools.

Contributor guide

Open the contributing guide

Research direction

Resolve the Fortran spelling RFC before implementation. Start with OMP.td, ClauseT.h, Clauses.cpp, OpenMP.cpp, and the parser, semantic, and lowering tests mentioned in the issue; compare the clang behavior and included reproducer. Done means the syntax is agreed, allowed-clause lists and parser/semantic/lowering support are covered, and the generated AMDGPU attributes match clang.

Written by the indexing model from the issue text.

Assessment

Tech stack
fortran
Domain
compilers
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.