[PATCH] D155633: [OpenMP][OpenMPIRBuilder] Add kernel launch codegen to emitTargetCall

Jan Sjödin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 11 09:00:44 PDT 2023


jsjodin marked 5 inline comments as done.
jsjodin added a comment.

Marked fixed items, one more to do that I didn't get to.



================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:5066-5073
+    if (!arg->getType()->isPointerTy()) {
+      combinedInfo.BasePointers.clear();
+      combinedInfo.Pointers.clear();
+      combinedInfo.Sizes.clear();
+      combinedInfo.Types.clear();
+      combinedInfo.Names.clear();
+      return;
----------------
TIFitis wrote:
> Given the args are added by this tests themselves, I think this condition is not needed.
> Given the args are added by this tests themselves, I think this condition is not needed.

Sure, I will remove the if.


================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:5077-5078
+    uint32_t SrcLocStrSize;
+    combinedInfo.Names.emplace_back(ompBuilder.getOrCreateSrcLocStr(
+        "Unknown loc - stub implementation", SrcLocStrSize));
+    combinedInfo.Types.emplace_back(llvm::omp::OpenMPOffloadMappingFlags(
----------------
TIFitis wrote:
> Maybe we can replace this with OMPBuilder.getOrCreateDefaultSrcLocStr?
> 
> Also it is fine to leave the Names empty if not available.
I'm leaving it for now since it doesn't affect the test.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155633/new/

https://reviews.llvm.org/D155633



More information about the llvm-commits mailing list