rjmccall added a comment. Thanks. Could you add some tests that `nontemporal` directives aren't disturbed by either (1) nested `nontemporal` directives or (2) nested directives of some other kind?
================ Comment at: clang/lib/CodeGen/CGExpr.cpp:3952 + CGM.getOpenMPRuntime().isNontemporalDecl(Field)) || + (!E->isArrow() && BaseLV.isNontemporal())) + LV.setNontemporal(/*Value=*/true); ---------------- ABataev wrote: > rjmccall wrote: > > ABataev wrote: > > > ABataev wrote: > > > > 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;` > > > > Similarly, what's the case in which the declaration is marked > > > > non-temporal and you *don't* want to trust that? > > > > > > There is no such case. We can mark `this->member` as nontemporal or > > > `declref`. The first check here checks if we have `this->member` marked > > > as nontemporal, the second check just propagates the flag if the base is > > > marked as nontemporal. > > Okay. Then this should just be `(BaseLV.isNontemporal() || > > CGM.getOpenMPRuntime().isNontemporalDecl(Field))`. > Still, `a->c` will be marked as nontemporal if `a` is nontemporal. Also, if > we remove the check for the `CXXThis` in the condition, we can erroneously > mark the instruction as nontemporal if we reference member of another base, > which is nontemporal: > ``` > struct S; > extern S s; > struct S { > int a, b; > void foo() { > #pragma omp simd nontemporal(a) > for (int i = 0; i < 10; ++i) > s.a = 0; // Will be marked as nontemporal though it should not? > } > } s; > > ``` > Still, a->c will be marked as nontemporal if a is nontemporal. No, the base LValue we build in this case will not be related to the LValue for `a`; the pointer is loaded from that l-value, but its properties do not propagate to the pointer, just like how `S * volatile s` doesn't mean that `s->x` is `volatile`. > Also, if we remove the check for the CXXThis in the condition, we can > erroneously mark the instruction as nontemporal if we reference member of > another base, which is nontemporal: I see, this is specifically part of the semantics that the field only becomes non-temporal for `this`. Okay, I accept that part of the original condition. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71708/new/ https://reviews.llvm.org/D71708 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits