[Openmp-commits] [PATCH] D105990: [OpenMP][NFC] Simplify targetDataEnd conditions for CopyMember

[George Rokos via Phabricator via Openmp-commits]

grokos added a comment.

I think we can simplify things even further. I have inlined my reasoning. Please double check in case I missed something.



================
Comment at: openmp/libomptarget/src/omptarget.cpp:719-720
         bool CopyMember = false;
         if (!(PM->RTLs.RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY) ||
             HasCloseModifier) {
+          if (IsLast)
----------------
`cond1`: This condition is useless because... see below.


================
Comment at: openmp/libomptarget/src/omptarget.cpp:726-727
         if ((DelEntry || Always || CopyMember) &&
             !(PM->RTLs.RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY &&
               TgtPtrBegin == HstPtrBegin)) {
           DP("Moving %" PRId64 " bytes (tgt:" DPxMOD ") -> (hst:" DPxMOD ")\n",
----------------
`cond2`: This is equivalent to `cond1`:

`!(PM->RTLs.RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY && TgtPtrBegin == HstPtrBegin)` is equivalent to
`!(PM->RTLs.RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY) || TgtPtrBegin != HstPtrBegin`.

`TgtPtrBegin != HstPtrBegin` if USM is not enabled OR if we have used the `close` modifier to allocate data on the device instead of host RAM, so:
`TgtPtrBegin != HstPtrBegin` is equivalent to
`!(PM->RTLs.RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY) || HasCloseModifier==true`.

It follows that `cond2` is equivalent to:
`!(PM->RTLs.RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY) || HasCloseModifier`
which is `cond1`.

`DelEntry`, `Always` and `CopyMember` are relevant only if `cond2` is true. So we can delete the `if(cond1)` above and leave only this bit:
```
if (IsLast)
  CopyMember = true;
```
But then, this would be equal to setting `CopyMember = IsLast`:
  - If `IsLast` is false, then `CopyMember` will be false (that's how it was declare a few lines above`.
  - If `IsLast` is true, `CopyMember` is set to true, which is the value of `IsLast`.
In all cases, `CopyMember = IsLast`.
So we can get rid of `CopyMember` entirely and leave the code like this:
```
if (ArgTypes[I] & OMP_TGT_MAPTYPE_FROM) {
  bool Always = ArgTypes[I] & OMP_TGT_MAPTYPE_ALWAYS;

  if ((DelEntry || Always || IsLast) &&
      !(PM->RTLs.RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY &&
        TgtPtrBegin == HstPtrBegin)) {
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105990/new/

https://reviews.llvm.org/D105990