[llvm-branch-commits] [clang] [llvm] [mlir] [OMPIRBuilder] Introduce struct to hold default kernel teams/threads (PR #116050)

[Sergio Afonso via llvm-branch-commits]

================
@@ -6182,9 +6182,12 @@ TEST_F(OpenMPIRBuilderTest, TargetRegion) {
 
   TargetRegionEntryInfo EntryInfo("func", 42, 4711, 17);
   OpenMPIRBuilder::LocationDescription OmpLoc({Builder.saveIP(), DL});
-  OpenMPIRBuilder::InsertPointOrErrorTy AfterIP = OMPBuilder.createTarget(
-      OmpLoc, /*IsOffloadEntry=*/true, Builder.saveIP(), Builder.saveIP(),
-      EntryInfo, -1, 0, Inputs, GenMapInfoCB, BodyGenCB, SimpleArgAccessorCB);
+  OpenMPIRBuilder::TargetKernelDefaultAttrs DefaultAttrs = {
+      /*MaxTeams=*/{-1}, /*MinTeams=*/0, /*MaxThreads=*/{0}, /*MinThreads=*/0};
----------------
skatrak wrote:

Defaults in the new struct represent basically what you would expect: max values representing "unset" (since these can be either unset (<0), runtime-evaluated (0) or constant (>0)) and min values set to 1. I believe that set of defaults makes sense, and it matches what clang set the corresponding attributes initially too.

As for not overriding the defaults in these tests, `MaxThreads < 0` causes the OMPIRBuilder to query the default grid size based on the target triple, whereas 0 won't. Querying that triggers an assert if the triple is not one of the supported offloading targets, so at least that one attribute can't be left unchanged unless we change the target triple of the OMPIRBuilder too. But, more generally, I think there is nothing in this PR that causes a need to update these tests, so I just set all of the values to what they already were before the struct was introduced rather than adapting them to its defaults.

I hope that makes sense to you, but let me know if you don't agree.

https://github.com/llvm/llvm-project/pull/116050