ABataev added inline comments.
================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:2953-2954 + (!(isOpenMPTargetExecutionDirective(DKind) && Res && + (*Res == OMPDeclareTargetDeclAttr::MT_Link || + *Res == OMPDeclareTargetDeclAttr::MT_To)))) { + // Only check for data-mapping attribute and is_device_ptr here ---------------- cchen wrote: > ABataev wrote: > > ABataev wrote: > > > Why comparing the value of `*Res` here if just comparing against all > > > possible values? This condition is always `true` if `Res` is true. I > > > don't think we need to map variables with `to` mapping. > > Do we need to map `declare target to` vars at all? > I don't understand this since spec says that > ``` > each variable referenced in the construct that does not have a predetermined > data-sharing attribute and does not appear in a to or link clause on a > declare target directive must be listed in a data-mapping attribute clause, a > data-sharing attribute clause (including a data-sharing attribute clause on a > combined construct where target is one of the constituent constructs), or an > is_device_ptr clause. > ``` > So, I think I should check both to and link. > Or are you saying that to clause is impossible to happen here since we have a > "Skip internally declared static variables" check so that if there is a to > clause, it will return before hitting Line 2952. Therefore, we should only > check for link clause? Missed `!` in the condition. Why we check for `!isOpenMPTargetExecutionDirective(DKind)` ================ Comment at: clang/lib/Sema/SemaOpenMP.cpp:2952 + VarsWithInheritedDSA.count(VD) == 0 && + (!(isOpenMPTargetExecutionDirective(DKind) && Res))) { + // Only check for data-mapping attribute and is_device_ptr here ---------------- Why extra parens in the condition? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69204/new/ https://reviews.llvm.org/D69204 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits