[PATCH] D71708: [OPENMP50]Codegen for nontemporal clause.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev marked 3 inline comments as done.
ABataev added inline comments.


================
Comment at: clang/lib/AST/StmtProfile.cpp:777
+    if (E)
+      Profiler->VisitStmt(E);
+  }
----------------
rjmccall wrote:
> Can `E` actually be null here?
No, remnants of the initial version.


================
Comment at: clang/lib/CodeGen/CGExpr.cpp:3952
+           CGM.getOpenMPRuntime().isNontemporalDecl(Field)) ||
+          (!E->isArrow() && BaseLV.isNontemporal()))
+        LV.setNontemporal(/*Value=*/true);
----------------
rjmccall wrote:
> Is the `!E->isArrow()` condition necessary here?  Why not just propagate the non-temporality of the base?
> 
> Similarly, what's the case in which the declaration is marked non-temporal and you *don't* want to trust that?
That's the main question. I try to mimic the behavior we currenty have in the codegen. If the lvalue for the pointer is marked as nontemporal, only loads/stores for the pointer itself are marked as nontemporal. Operations with the memory it points to are not marked as nontemporal. I'm trying to do the same. E.g., if have something like `a.b->c` and `a` is nontemporal, then only `a.b = x;` must be marked as nontemporal, but not `a.b->c = x;`


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:11349
+  NontemporalDeclsSet &DS =
+      CGM.getOpenMPRuntime().NontemporalDeclsStack.emplace_back();
+  // No need to check for nontemporal clauses in non-simd directives.
----------------
rjmccall wrote:
> This is effectively clearing the active non-temporal decls set.  Is that okay (even for the non-SIMD directives?), or should the current set be copied?
Yes, it must be copied, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71708