ABataev added inline comments.

================
Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:246-249
+          if (!IsVMTTy)
+            EscapedParameters.insert(VD);
+          else if (!IsForCombinedParallelRegion)
+            return;
----------------
Also, can you make it specific to pinter types?


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:243
         if (!FD->getType()->isReferenceType()) {
-          assert(!VD->getType()->isVariablyModifiedType() &&
+          bool isVMT = VD->getType()->isVariablyModifiedType();
+          assert((!isVMT || (isVMT && !IsForCombinedParallelRegion)) &&
----------------
doru1004 wrote:
> doru1004 wrote:
> > ABataev wrote:
> > > ABataev wrote:
> > > > doru1004 wrote:
> > > > > doru1004 wrote:
> > > > > > ABataev wrote:
> > > > > > > Is it for pointers only? Or for other types too?
> > > > > > I am not sure about that distinction; it allows for the same types 
> > > > > > as before except that when the directive is not a combined parallel 
> > > > > > for directive it returns immediately because the variable will then 
> > > > > > have to be captured and shared.
> > > > > Correction: it allows for VMTs to pass through in cases in which you 
> > > > > don't use a combined parallel for directive.
> > > > `IsVMTTy`?
> > > How can it be captured by value? Why does it happen? My first question 
> > > relates to the assertion you changed. I assume, it applies only to the 
> > > pointers. Otherwise, please add more context how we can capture mariably 
> > > modifiable type by value.
> > Yes it should be called `IsVMTTy`
> It only applies to pointers, it's the pointer that needs to be used correctly.
Can you change the assert check then to something like this:
```
assert((VD->getType()->isPointerType() || 
!VD->getType()->isVariablyModifiedType()) && 
```


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

https://reviews.llvm.org/D138614

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to