[Mlir-commits] [flang] [llvm] [mlir] [Flang][OpenMP] Update MapInfoFinalization to use BlockArgs Interface and modify use_device_ptr/addr to be order independent (PR #113919)

[Akash Banerjee]

================
@@ -2351,25 +2350,22 @@ static void collectMapDataFromMapOperands(
       Value offloadPtr =
           mapOp.getVarPtrPtr() ? mapOp.getVarPtrPtr() : mapOp.getVarPtr();
       llvm::Value *origValue = moduleTranslation.lookupValue(offloadPtr);
-
-      // Check if map info is already present for this entry.
-      if (!findMapInfo(origValue, devInfoTy)) {
-        mapData.OriginalValue.push_back(origValue);
-        mapData.Pointers.push_back(mapData.OriginalValue.back());
-        mapData.IsDeclareTarget.push_back(false);
-        mapData.BasePointers.push_back(mapData.OriginalValue.back());
-        mapData.BaseType.push_back(
-            moduleTranslation.convertType(mapOp.getVarType()));
-        mapData.Sizes.push_back(builder.getInt64(0));
-        mapData.MapClause.push_back(mapOp.getOperation());
-        mapData.Types.push_back(
-            llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_RETURN_PARAM);
-        mapData.Names.push_back(LLVM::createMappingInformation(
-            mapOp.getLoc(), *moduleTranslation.getOpenMPBuilder()));
-        mapData.DevicePointers.push_back(devInfoTy);
-        mapData.IsAMapping.push_back(false);
-        mapData.IsAMember.push_back(checkIsAMember(useDevOperands, mapOp));
-      }
+      llvm::omp::OpenMPOffloadMappingFlags mapType =
----------------
TIFitis wrote:

I'm a but vague on how this is necessary for ordering purposes but I can imagine how it might be needed.


Let me share my perspective:

UseDevicePtr/Addr is really just like enabling an extra flag or mapTypeMpdifier for the mapClause. It should really just be part of MapType or the map clause itself but it's not because only a subset of the directives that allow the map clause also allow the useDevPtr/Addr.

The expected/common behavior is that a variable which occurs in a UseDev clause has already been mapped in one of the MapClause. However, this isn't a rule, hence when we encounter a UseDev clause variable with no previous MapInfo for it, we create a "dummy" mapping for it. This is more in the category "no idea what the user wants here, let he/she bother".

As such, generating separate MapInfo for each and every UseDev clause doesn't really make sense. Moreover, it is only recently we switched the UseDev clauses to make use of the MapOps but that was only to leverage the vivid data type support of the mapOp and nothing else.

With this philosophy in mind I don't think we should always be adding MapInfo for each UseDev, as they are a lack of information in themselves and hold no information wortthy of preservation.

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