[llvm-branch-commits] [flang] [Flang][OpenMP] Update flang with changes to the OpenMP dialect (PR #92524)

Sergio Afonso via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Wed Jun 12 04:33:07 PDT 2024


================
@@ -1086,8 +1086,9 @@ static void genTargetDataClauses(
   // ordering.
   // TODO: Perhaps create a user provideable compiler option that will
   // re-introduce a hard-error rather than a warning in these cases.
-  promoteNonCPtrUseDevicePtrArgsToUseDeviceAddr(clauseOps, useDeviceTypes,
-                                                useDeviceLocs, useDeviceSyms);
+  promoteNonCPtrUseDevicePtrArgsToUseDeviceAddr(
+      clauseOps.useDeviceAddrVars, clauseOps.useDevicePtrVars, useDeviceTypes,
+      useDeviceLocs, useDeviceSyms);
----------------
skatrak wrote:

The reason I made this change is that otherwise we'd have to change the `clauseOps` argument type received by the function to `TargetDataClauseOps`, which I feel is a bit too broad. I know `target` is currently the only construct that uses these clauses, but even if that's always going to be the case, it seems wrong to pass all `target` information to a function that only cares about `use_device_ptr` and `use_device_addr` clauses.

Since information for these clauses is now split into independent structures, if we wanted to pass them as we did before without some unrelated `target` information (which would also restrict the function to only possibly ever apply to that construct), we'd have to pass both a `UseDevicePtrClauseOps` and a `UseDeviceAddrClauseOps` object reference. At that point passing structures made little sense and it was too verbose, and that's why I decided that just passing the lists would made a bit more sense.

So, that's the thinking behind this change. It boils down to prioritizing the "principle" of what should be an argument of that function over what would reduce the size of the argument list by one element. I'm happy to change it back, though, if the general consensus is to do it the other way around. Just sharing my thought process here.

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


More information about the llvm-branch-commits mailing list