jdenny marked an inline comment as done.
jdenny added inline comments.
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7948-7949
+ MapFlagsArrayTy &Types,
+ const llvm::DenseSet<const ValueDecl *> &SkipVarSet =
+ llvm::DenseSet<const ValueDecl *>()) const {
// We have to process the component lists that relate with the same
----------------
ABataev wrote:
> Use `llvm::DenseSet<CanonicalDeclPtr<const ValueDecl>> &SkipVarSet`.
Thanks for the suggestion.
`CanonicalDeclPtr<const ValueDecl>` appears to be unusable. The constructor
(in `Redeclarable.h`) tries to initialize the `const ValueDecl *Ptr` member
with the `Decl *` returned by `getCanonicalDecl`, producing a type mismatch.
I went with `CanonicalDeclPtr<const Decl>` instead.
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9468-9469
CurSizes, CurMapTypes, PartialStruct);
+ if (!CI->capturesThis())
+ MappedVarSet.insert(CI->getCapturedVar()->getCanonicalDecl());
+ else
----------------
ABataev wrote:
> I would check that we capture a variable. We may capture VLA here as well.
> I would check that we capture a variable. We may capture VLA here as well.
The `if` on line 9454 above checks for VLAs. This code is in the `else`.
Similarly, the existing implementation for `generateInfoForCapture`, which is
called within this same `else`, doesn't protect against VLAs. It just checks
`capturesThis` as I'm doing there.
Have I misunderstood?
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9470-9471
+ MappedVarSet.insert(CI->getCapturedVar()->getCanonicalDecl());
+ else
+ MappedVarSet.insert(nullptr);
if (CurBasePointers.empty())
----------------
ABataev wrote:
> No need to insert `nullptr` here, it is not used later.
Without this `nulltpr` insertion, many tests fail because of duplicate map
entries. Independently of my patch, `nulltptr` is used to indicate `this`
capture (see the `generateInfoForCapture` implementation) .
For example:
```
struct S {
int i;
void foo() {
#pragma omp target map(i)
i = 5;
}
};
```
We should have:
```
@.offload_maptypes = private unnamed_addr constant [2 x i64] [i64 32, i64
281474976710659]
```
Without the `nullptr` insertion, we have:
```
@.offload_maptypes = private unnamed_addr constant [4 x i64] [i64 32, i64
281474976710659, i64 32, i64 844424930131971]
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83922/new/
https://reviews.llvm.org/D83922
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
