sfantao added a comment.
Hi Alexey,
Thanks for the review!
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:4393-4431
@@ +4392,41 @@
+ /// retrieved from the provided map clause expression.
+ DeclarationMapInfoEntry(const Expr *MCE, OpenMPMapClauseKind MapType,
+ OpenMPMapClauseKind MapTypeModifier)
+ : MapType(MapType), MapTypeModifier(MapTypeModifier) {
+ assert(MCE && "Invalid expression??");
+ while (true) {
+ MCE = MCE->IgnoreParenImpCasts();
+
+ if (auto *CurE = dyn_cast<DeclRefExpr>(MCE)) {
+ Components.push_back(
+ ComponentTy(CurE, cast<VarDecl>(CurE->getDecl())));
+ break;
+ }
+
+ if (auto *CurE = dyn_cast<MemberExpr>(MCE)) {
+ auto *BaseE = CurE->getBase()->IgnoreParenImpCasts();
+
+ Components.push_back(
+ ComponentTy(CurE, cast<FieldDecl>(CurE->getMemberDecl())));
+ if (isa<CXXThisExpr>(BaseE))
+ break;
+
+ MCE = BaseE;
+ continue;
+ }
+
+ if (auto *CurE = dyn_cast<ArraySubscriptExpr>(MCE)) {
+ Components.push_back(ComponentTy(CurE, nullptr));
+ MCE = CurE->getBase()->IgnoreParenImpCasts();
+ continue;
+ }
+
+ if (auto *CurE = dyn_cast<OMPArraySectionExpr>(MCE)) {
+ Components.push_back(ComponentTy(CurE, nullptr));
+ MCE = CurE->getBase()->IgnoreParenImpCasts();
+ continue;
+ }
+
+ llvm_unreachable("Invalid map clause expression!");
+ }
+ }
----------------
ABataev wrote:
> I do recall that similar code already exists in Sema. Maybe you'd better to
> store info about top-level bases in map clause in Sema rather than copy the
> code between modules?
Yes, Sema does something similar. In the last diff I use what Sema produces and
store it in the clause. The implementation is in the dependence diff.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:4462
@@ +4461,3 @@
+ if (const auto *OAE = dyn_cast<OMPArraySectionExpr>(E)) {
+ auto BaseTy = OMPArraySectionExpr::getBaseOriginalType(
+ OAE->getBase()->IgnoreParenImpCasts())
----------------
ABataev wrote:
> Replace 'auto' by 'QualType'. Also, I think all such deep digging into
> structure of expressions must reside in Sema rather than in codegen and info
> must be stored within OMPMapClause
Done. The information is now stored in the map clause.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:4493-4498
@@ +4492,8 @@
+
+ /// \brief Generate the address of the lower bound of the section defined by
+ /// expression \a E.
+ llvm::Value *getLowerBoundOfElement(const Expr *E) const {
+ return CGF.EmitLValue(E).getPointer();
+ }
+
+ /// \brief Return the corresponding bits for a given map clause modifier. Add
----------------
ABataev wrote:
> Do we really need this? It is quite simple and has nothing to do with the
> name of the function
Ok, I removed this.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:4681-4684
@@ +4680,6 @@
+ if (I->second->getType()->isAnyPointerType() && std::next(I) != CE) {
+ auto PtrAddr =
+ CGF.MakeNaturalAlignAddrLValue(BP, I->second->getType());
+ BP = CGF.EmitLoadOfLValue(PtrAddr, SourceLocation()).getScalarVal();
+
+ // We do not need to generate individual map information for the
----------------
ABataev wrote:
> CGF has EmitLoadOfPointer() function
Ok, using `EmitLoadOfPointerLValue` now.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:4699-4735
@@ +4698,39 @@
+
+ // A final array section, is one whose length can't be proved to be
one.
+ auto IsFinalArraySection = [this](const Expr *E) -> bool {
+ auto *OASE = dyn_cast<OMPArraySectionExpr>(E);
+
+ // It is not an array section and therefore not a unity-size one.
+ if (!OASE)
+ return false;
+
+ // An array section with no colon always refer to a single element.
+ if (OASE->getColonLoc().isInvalid())
+ return false;
+
+ auto *Length = OASE->getLength();
+
+ // If we don't have a length we have to check if the array has size 1
+ // for this dimension. Also, we should always expect a length if the
+ // base type is pointer.
+ if (!Length) {
+ auto BaseQTy = OMPArraySectionExpr::getBaseOriginalType(
+ OASE->getBase()->IgnoreParenImpCasts())
+ .getCanonicalType();
+ if (auto *ATy = dyn_cast<ConstantArrayType>(BaseQTy.getTypePtr()))
+ return ATy->getSize().getSExtValue() != 1;
+ // If we don't have a constant dimension length, we have to
consider
+ // the current section as having any size, so it is not necessarily
+ // unitary. If it happen to be unity size, that's user fault.
+ return true;
+ }
+
+ // Check if the length evaluates to 1.
+ llvm::APSInt ConstLength;
+ if (!Length->EvaluateAsInt(ConstLength, CGF.getContext()))
+ return true; // Can have more that size 1.
+
+ return ConstLength.getSExtValue() != 1;
+ }(I->first);
+
+ // Get information on whether the element is a pointer. Have to do a
----------------
ABataev wrote:
> Maybe it is better to extract it as a member function?
Moved this code to a new member function as suggested.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:4700
@@ +4699,3 @@
+ // A final array section, is one whose length can't be proved to be
one.
+ auto IsFinalArraySection = [this](const Expr *E) -> bool {
+ auto *OASE = dyn_cast<OMPArraySectionExpr>(E);
----------------
ABataev wrote:
> auto &&
I am now using a member function as you suggested below.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:4794
@@ +4793,3 @@
+ for (auto *MC : Directive.getClausesOfKind<OMPMapClause>()) {
+ for (auto *RE : MC->getVarRefs()) {
+ auto *MI = new DeclarationMapInfoEntry(RE, MC->getMapType(),
----------------
ABataev wrote:
> getVarRefs()->varlists(). getVarRefs() is protected
I am doing as you say. In any case, the const variant of `getVarRefs` is
currently public member. Maybe that is something you plan to fix?
http://reviews.llvm.org/D16749
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
