[PATCH] D92189: [OpenMPIRBuilder] forward arguments as pointers to outlined function

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 30 21:48:23 PST 2020


jdoerfert added inline comments.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:736
              "Expected copy/create callback to set replacement value!");
-      if (ReplacementValue == &V)
-        return;
     }
 
----------------
jdoerfert wrote:
> I was expecting the above logic to be placed here, after the `PrivCB` callback as I assumed we would privatize in the sequential part. Clang does not and we do not either (for now), which is unfortunate. It does duplicate the privatization and makes this patch more complicated than I anticipated. I though we can simply replace `ReplacementValue` by `Reloaded` if `ReplacementValue` wasn't a pointer and therefor put in an alloca. The fact that we need to reload `V` and then "replace twice" seems really unfortunate. 
> 
> What would happen if you do not have `Reloaded` at all but instead make it `V = createlodoad(alloca)`? After all, `Reloaded` is equivalent to `V` in all aspects, right? Would this work? Would we still need the code below? I feel like there must be a minimal solution as we only want to replace the value once on the other side of the call edge.
-"duplicate privatization" +"duplicate replace all uses with"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92189



More information about the llvm-commits mailing list