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

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 19 20:30:33 PST 2019


rjmccall added inline comments.


================
Comment at: clang/lib/AST/StmtProfile.cpp:777
+    if (E)
+      Profiler->VisitStmt(E);
+  }
----------------
Can `E` actually be null here?


================
Comment at: clang/lib/CodeGen/CGExpr.cpp:3952
+           CGM.getOpenMPRuntime().isNontemporalDecl(Field)) ||
+          (!E->isArrow() && BaseLV.isNontemporal()))
+        LV.setNontemporal(/*Value=*/true);
----------------
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?


================
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.
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71708





More information about the cfe-commits mailing list