ABataev marked an inline comment as done.
ABataev 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:
> > 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;
```
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
