[flang-commits] [flang] [OpenMP][Flang] Enable alias analysis inside omp target region (PR #111670)

[via flang-commits]

================
@@ -319,6 +330,32 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
           breakFromLoop = true;
         })
         .Case<hlfir::DeclareOp, fir::DeclareOp>([&](auto op) {
+          // If declare operation is inside omp target region,
+          // continue alias analysis outside the target region
+          if (llvm::isa<omp::TargetOp>(op->getParentOp())) {
+            omp::TargetOp targetOp =
+                llvm::cast<omp::TargetOp>(op->getParentOp());
+            auto mapClauseOwner =
+                llvm::dyn_cast<omp::MapClauseOwningOpInterface>(
+                    targetOp.getOperation());
+            if (!mapClauseOwner) {
+              defOp = nullptr;
+              breakFromLoop = true;
+              return;
+            }
+            Block *targetEntryBlock = &targetOp->getRegion(0).front();
+            OperandRange mapVarsArr = mapClauseOwner.getMapVars();
+            assert(mapVarsArr.size() == targetEntryBlock->getNumArguments());
----------------
agozillon wrote:

@skatrak will know better than I do, but I am not sure this check (the subsequent loop may also have some issues, but at a glance I doubt it) will be correct soon, as I believe there's perhaps other clauses that can end up appending block arguments to target operations (other than use_device_addr/ptr) and I believe @skatrak is refactoring this in another PR or plans to. I could be incorrect however and Sergio can correct me (which would be a good learning experience for me if so, not had a chance to try the new method just yet)! 

But in either case, might not be applicable just yet and it's more just something to keep in mind depending on patch landing order!

Also, dumb but maybe interesting question, but does this lack of alias information also apply to TargetDataOp, it's a host side construct but from what I recall it also has a region with the host code wrapped by the target data inside of it, however, I do not believe it is IsolatedFromAbove. So this may or may not also run into a very similar issue where we are blocking optimisations by not proliferating information. This also opens up a bit more of a complex case where you can have a target data directive that wraps one or more target directives. But just food for thought while we're considering missed optimisation opportunities as target data is often over looked (by me at least) and works surprisingly similarly in a lot of ways.  

And if we end up looking into TargetDataOp, it'd be good to test this little blob of code with use_device_addr/ptr as well, as they function similarly by generating map info (at least downstream currently, unsure if they've made the journey upstream just yet...), should still be accessible via the map clause owner interface, just good to make sure nothing breaks!

And to be clear not suggesting this be done in this PR, just curious (and just came to mind) as it'd be interesting if we were blocking host side optimisations via target data too  :-)

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