rjmccall added inline comments.
================
Comment at: clang/lib/CodeGen/CGExpr.cpp:3952
+ CGM.getOpenMPRuntime().isNontemporalDecl(Field)) ||
+ (!E->isArrow() && BaseLV.isNontemporal()))
+ LV.setNontemporal(/*Value=*/true);
----------------
rjmccall wrote:
> 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.
But please add some tests that you don't honor `nontemporal` on base
expressions not derived from `this`.
Does "wrapped" include looking through derived-to-base conversions? You should
test that as well.
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
