================
@@ -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
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

Reply via email to