This is an archive of the discontinued LLVM Phabricator instance.

[OpenMPIRBuilder] Migrate emitOffloadingArraysArgument from clang
ClosedPublic

Authored by jsjodin on Sep 26 2022, 11:50 AM.

Details

Summary

Move emitOffloadingArraysArgument function and supporting data structures to OpenMPIRBuilder. This will later be used in flang as well.
The TargetDataInfo class was split up into generic information and clang-specific data, which remain in clang. Further migration will be done in
in the future.

Diff Detail

Event Timeline

jsjodin created this revision.Sep 26 2022, 11:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 26 2022, 11:50 AM
jsjodin requested review of this revision.Sep 26 2022, 11:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 26 2022, 11:50 AM

Initial feedback, mostly style.

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
1026 ↗(On Diff #462975)

No llvm:: in llvm/

1044 ↗(On Diff #462975)

Seems unused. If needed in can live in Clang.

1053–1059 ↗(On Diff #462975)

RTArgs = TargetDataRTArgs()?

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
3997–4001 ↗(On Diff #462975)

These types should exist but I guess this is OK.

4002 ↗(On Diff #462975)

Flip the cases, simple first, then exit.

4026 ↗(On Diff #462975)

!EmitDebug

jsjodin updated this revision to Diff 463552.Sep 28 2022, 7:31 AM

Fixed issues identified in review.

jsjodin marked 5 inline comments as done.Sep 28 2022, 7:39 AM

Updated the code for everthing except for type creation, which will be fixed shortly.

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
1044 ↗(On Diff #462975)

Yes, this is dead code I forgot to remove.

1053–1059 ↗(On Diff #462975)

Yes, much better.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
3997–4001 ↗(On Diff #462975)

It can be compressed by using PointerType *VoidPtrType = Type::getInt8PtrTy(C); I will also fix this, but in the next patch update.

4002 ↗(On Diff #462975)

Did an early return.

4026 ↗(On Diff #462975)

Test and call site were both swapped, fixed both.

jsjodin updated this revision to Diff 463583.Sep 28 2022, 9:19 AM
jsjodin marked 4 inline comments as done.

Use getInt8PtrTy.

jsjodin marked an inline comment as done.Sep 28 2022, 9:22 AM
jsjodin added inline comments.
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
3997–4001 ↗(On Diff #462975)

Did minor cleanup.

jdoerfert accepted this revision.Sep 28 2022, 10:09 AM

LG, nits below

include/llvm/Frontend/OpenMP/OMPIRBuilder.h
1027

Move to the bool.

lib/Frontend/OpenMP/OMPIRBuilder.cpp
4010–4012

also below.

This revision is now accepted and ready to land.Sep 28 2022, 10:09 AM
jsjodin updated this revision to Diff 463887.Sep 29 2022, 7:23 AM

Moved comment and removed llvm:: uses.

jsjodin updated this revision to Diff 465533.Oct 5 2022, 1:30 PM

Added IRBuilder test.

jsjodin updated this revision to Diff 465721.Oct 6 2022, 7:20 AM

Fix formatting.

jsjodin updated this revision to Diff 465829.Oct 6 2022, 12:30 PM

Use standard patch format.

Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2022, 5:04 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript