Skip to content

Conversation

skatrak
Copy link
Member

@skatrak skatrak commented Sep 17, 2024

This patch adds missing MLIR to LLVM IR translation support for the if clause on omp.target operations. This completes the missing piece for Fortran support of !$omp target if(...).

The implementation updates emitTargetCall() in the OMPIRBuilder to follow clang's support for the if clause in CGOpenMPRuntime::emitTargetCall().

…M IR

This patch adds missing MLIR to LLVM IR translation support for the `if` clause
on `omp.target` operations. This completes the missing piece for Fortran
support of `!$omp target if(...)`.

The implementation updates `emitTargetCall()` in the OMPIRBuilder to follow
clang's support for the `if` clause in  `CGOpenMPRuntime::emitTargetCall()`.
Copy link

@mjklemm mjklemm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I have had a hard time following the changes as the diff was sort of messed up. So, second pair of eyes might be useful.

@skatrak
Copy link
Member Author

skatrak commented Sep 18, 2024

LGTM, but I have had a hard time following the changes as the diff was sort of messed up. So, second pair of eyes might be useful.

Yes, it flags a bunch of changes because I basically extracted most of the body of emitTargetCall into a couple of lambdas. The only real change is at the bottom of the function, where it's decided when to run each part. It's generally a simple change: just adding an if-else depending on the runtime value of the if expression, if present, otherwise it does the same thing it used to.

Copy link

@dpalermo dpalermo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified this changes works correctly on Leopold's reproducer.

@bhandarkar-pranav
Copy link

@skatrak thank you for the PR. LGTM.

@skatrak skatrak merged commit 748aa57 into ROCm:amd-trunk-dev Sep 19, 2024
3 of 5 checks passed
@skatrak skatrak deleted the target-if branch September 19, 2024 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants