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

No llvm:: in llvm/

1044

Seems unused. If needed in can live in Clang.

1053–1059

RTArgs = TargetDataRTArgs()?

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
3997–4001

These types should exist but I guess this is OK.

4002

Flip the cases, simple first, then exit.

4026

!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

Yes, this is dead code I forgot to remove.

1053–1059

Yes, much better.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
3997–4001

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

4002

Did an early return.

4026

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

Did minor cleanup.

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

LG, nits below

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

Move to the bool.

lib/Frontend/OpenMP/OMPIRBuilder.cpp
4007–4009 ↗(On Diff #463552)

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