This is an archive of the discontinued LLVM Phabricator instance.

[flang][OpenMP] Support for common block in copyin clause
ClosedPublic

Authored by NimishMishra on Aug 4 2023, 4:07 AM.

Details

Summary

This patch adds lowering support for threadprivate variables encapsulated in a common block and marked inside a copyin clause.

Diff Detail

Event Timeline

NimishMishra created this revision.Aug 4 2023, 4:07 AM
NimishMishra requested review of this revision.Aug 4 2023, 4:07 AM
flang/lib/Lower/OpenMP.cpp
1543–1544

Is a break or something missing here?

NimishMishra added inline comments.Aug 6 2023, 11:47 PM
flang/lib/Lower/OpenMP.cpp
1543–1544

Thank you for the spot. Indeed I missed. sym here represents the common block, and we need to copy host association details for all members of the block, and not just the last (which was what happened in previous iteration of the patch).

omp.parallel {
%[[val_6:.*]] = fir.alloca i32 {adapt.valuebyref, pinned}
%[[val_7:.*]] = omp.threadprivate %[[val_1]] : !fir.ref<!fir.array<8xi8>> -> !fir.ref<!fir.array<8xi8>>
%[[val_8:.*]] = fir.convert %[[val_7]] : (!fir.ref<!fir.array<8xi8>>) -> !fir.ref<!fir.array<?xi8>>
%[[val_c0:.*]] = arith.constant 0 : index
%[[val_9:.*]] = fir.coordinate_of %[[val_8]], %[[val_c0]] : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%[[val_10:.*]] = fir.convert %[[val_9]] : (!fir.ref<i8>) -> !fir.ref<i32>
%[[val_11:.*]] = fir.convert %[[val_7]] : (!fir.ref<!fir.array<8xi8>>) -> !fir.ref<!fir.array<?xi8>>
%[[val_c4_0:.*]] = arith.constant 4 : index
%[[val_12:.*]] = fir.coordinate_of %[[val_11]], %[[val_c4_0]] : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
%[[val_13:.*]] = fir.convert %[[val_12]] : (!fir.ref<i8>) -> !fir.ref<i32>
%[[val_14:.*]] = fir.load %[[val_5]] : !fir.ref<i32>
fir.store %[[val_14]] to %[[val_13]] : !fir.ref<i32>
omp.barrier

This was the IR generated when both x and y were members of the common block. As evident, the conversion happens for both indices 0 and 4 (since both x and y are integers), but the fir.store happens only for y, and not for x.

The present version of the patch does it for both. Please have a look.

This revision is now accepted and ready to land.Aug 7 2023, 1:45 AM

Hi @kiranchandramohan

Thanks for the LG. Small hiccup. The status here shows build failed, but when I go to https://buildkite.com/llvm-project/diff-checks/builds/190652, both linux and windows builds have passed. Can I go ahead with the merge? Seems like a buildbot issue.

This revision was landed with ongoing or failed builds.Aug 20 2023, 9:34 PM
This revision was automatically updated to reflect the committed changes.