[OPENMP] Fix PR38903: Crash on instantiation of the non-dependent

declare reduction.

If the declare reduction construct with the non-dependent type is
defined in the template construct, the compiler might crash on the
template instantition. Reworked the whole instantiation scheme for the
declare reduction constructs to fix this problem correctly.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@342151 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/DeclOpenMP.h b/include/clang/AST/DeclOpenMP.h
index 1f2aaa151e4be7b64719f92ee64bf4755e71584e..33fe0e196dc13531351042b3b7f40fc1f96dbbee 100644 (file)
--- a/include/clang/AST/DeclOpenMP.h
+++ b/include/clang/AST/DeclOpenMP.h
@@ -112,9 +112,17 @@ public:
 private:
   friend class ASTDeclReader;
   /// Combiner for declare reduction construct.
-  Expr *Combiner;
+  Expr *Combiner = nullptr;
   /// Initializer for declare reduction construct.
-  Expr *Initializer;
+  Expr *Initializer = nullptr;
+  /// In parameter of the combiner.
+  Expr *In = nullptr;
+  /// Out parameter of the combiner.
+  Expr *Out = nullptr;
+  /// Priv parameter of the initializer.
+  Expr *Priv = nullptr;
+  /// Orig parameter of the initializer.
+  Expr *Orig = nullptr;
 
   /// Reference to the previous declare reduction construct in the same
   /// scope with the same name. Required for proper templates instantiation if
@@ -143,8 +151,19 @@ public:
   /// Get combiner expression of the declare reduction construct.
   Expr *getCombiner() { return Combiner; }
   const Expr *getCombiner() const { return Combiner; }
+  /// Get In variable of the combiner.
+  Expr *getCombinerIn() { return In; }
+  const Expr *getCombinerIn() const { return In; }
+  /// Get Out variable of the combiner.
+  Expr *getCombinerOut() { return Out; }
+  const Expr *getCombinerOut() const { return Out; }
   /// Set combiner expression for the declare reduction construct.
   void setCombiner(Expr *E) { Combiner = E; }
+  /// Set combiner In and Out vars.
+  void setCombinerData(Expr *InE, Expr *OutE) {
+    In = InE;
+    Out = OutE;
+  }
 
   /// Get initializer expression (if specified) of the declare reduction
   /// construct.
@@ -154,11 +173,22 @@ public:
   InitKind getInitializerKind() const {
     return static_cast<InitKind>(OMPDeclareReductionDeclBits.InitializerKind);
   }
+  /// Get Orig variable of the initializer.
+  Expr *getInitOrig() { return Orig; }
+  const Expr *getInitOrig() const { return Orig; }
+  /// Get Priv variable of the initializer.
+  Expr *getInitPriv() { return Priv; }
+  const Expr *getInitPriv() const { return Priv; }
   /// Set initializer expression for the declare reduction construct.
   void setInitializer(Expr *E, InitKind IK) {
     Initializer = E;
     OMPDeclareReductionDeclBits.InitializerKind = IK;
   }
+  /// Set initializer Orig and Priv vars.
+  void setInitializerData(Expr *OrigE, Expr *PrivE) {
+    Orig = OrigE;
+    Priv = PrivE;
+  }
 
   /// Get reference to previous declare reduction construct in the same
   /// scope with the same name.

diff --git a/lib/CodeGen/CGOpenMPRuntime.cpp b/lib/CodeGen/CGOpenMPRuntime.cpp
index 6c37c6cf115b7a9dbd8c2cd977ec2127a75760fd..9452e964c15a74a550c2b745f649c921c86bd46f 100644 (file)
--- a/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -1295,27 +1295,19 @@ void CGOpenMPRuntime::emitUserDefinedReduction(
     CodeGenFunction *CGF, const OMPDeclareReductionDecl *D) {
   if (UDRMap.count(D) > 0)
     return;
-  ASTContext &C = CGM.getContext();
-  if (!In || !Out) {
-    In = &C.Idents.get("omp_in");
-    Out = &C.Idents.get("omp_out");
-  }
   llvm::Function *Combiner = emitCombinerOrInitializer(
-      CGM, D->getType(), D->getCombiner(), cast<VarDecl>(D->lookup(In).front()),
-      cast<VarDecl>(D->lookup(Out).front()),
+      CGM, D->getType(), D->getCombiner(),
+      cast<VarDecl>(cast<DeclRefExpr>(D->getCombinerIn())->getDecl()),
+      cast<VarDecl>(cast<DeclRefExpr>(D->getCombinerOut())->getDecl()),
       /*IsCombiner=*/true);
   llvm::Function *Initializer = nullptr;
   if (const Expr *Init = D->getInitializer()) {
-    if (!Priv || !Orig) {
-      Priv = &C.Idents.get("omp_priv");
-      Orig = &C.Idents.get("omp_orig");
-    }
     Initializer = emitCombinerOrInitializer(
         CGM, D->getType(),
         D->getInitializerKind() == OMPDeclareReductionDecl::CallInit ? Init
                                                                      : nullptr,
-        cast<VarDecl>(D->lookup(Orig).front()),
-        cast<VarDecl>(D->lookup(Priv).front()),
+        cast<VarDecl>(cast<DeclRefExpr>(D->getInitOrig())->getDecl()),
+        cast<VarDecl>(cast<DeclRefExpr>(D->getInitPriv())->getDecl()),
         /*IsCombiner=*/false);
   }
   UDRMap.try_emplace(D, Combiner, Initializer);
@@ -8052,19 +8044,19 @@ void CGOpenMPRuntime::scanForTargetRegionsFunctions(const Stmt *S,
 }
 
 bool CGOpenMPRuntime::emitTargetFunctions(GlobalDecl GD) {
-  const auto *FD = cast<FunctionDecl>(GD.getDecl());
-
   // If emitting code for the host, we do not process FD here. Instead we do
   // the normal code generation.
   if (!CGM.getLangOpts().OpenMPIsDevice)
     return false;
 
   // Try to detect target regions in the function.
-  scanForTargetRegionsFunctions(FD->getBody(), CGM.getMangledName(GD));
+  const ValueDecl *VD = cast<ValueDecl>(GD.getDecl());
+  if (const auto *FD = dyn_cast<FunctionDecl>(VD))
+    scanForTargetRegionsFunctions(FD->getBody(), CGM.getMangledName(GD));
 
   // Do not to emit function if it is not marked as declare target.
-  return !OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(FD) &&
-         AlreadyEmittedTargetFunctions.count(FD->getCanonicalDecl()) == 0;
+  return !OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD) &&
+         AlreadyEmittedTargetFunctions.count(VD->getCanonicalDecl()) == 0;
 }
 
 bool CGOpenMPRuntime::emitTargetGlobalVariable(GlobalDecl GD) {
@@ -8152,7 +8144,8 @@ void CGOpenMPRuntime::registerTargetGlobalVariable(const VarDecl *VD,
 }
 
 bool CGOpenMPRuntime::emitTargetGlobal(GlobalDecl GD) {
-  if (isa<FunctionDecl>(GD.getDecl()))
+  if (isa<FunctionDecl>(GD.getDecl()) ||
+      isa<OMPDeclareReductionDecl>(GD.getDecl()))
     return emitTargetFunctions(GD);
 
   return emitTargetGlobalVariable(GD);

diff --git a/lib/CodeGen/CGOpenMPRuntime.h b/lib/CodeGen/CGOpenMPRuntime.h
index 46fa774f2aec5b49722ef1e6de8829254d44697a..e0685d9bc66091db588aaab25099def1b9ca5808 100644 (file)
--- a/lib/CodeGen/CGOpenMPRuntime.h
+++ b/lib/CodeGen/CGOpenMPRuntime.h
@@ -315,10 +315,6 @@ private:
                          SmallVector<const OMPDeclareReductionDecl *, 4>>
       FunctionUDRMapTy;
   FunctionUDRMapTy FunctionUDRMap;
-  IdentifierInfo *In = nullptr;
-  IdentifierInfo *Out = nullptr;
-  IdentifierInfo *Priv = nullptr;
-  IdentifierInfo *Orig = nullptr;
   /// Type kmp_critical_name, originally defined as typedef kmp_int32
   /// kmp_critical_name[8];
   llvm::ArrayType *KmpCriticalNameTy;
@@ -600,7 +596,7 @@ private:
   OffloadEntriesInfoManagerTy OffloadEntriesInfoManager;
 
   bool ShouldMarkAsGlobal = true;
-  llvm::SmallDenseSet<const FunctionDecl *> AlreadyEmittedTargetFunctions;
+  llvm::SmallDenseSet<const Decl *> AlreadyEmittedTargetFunctions;
 
   /// List of variables that can become declare target implicitly and, thus,
   /// must be emitted.

diff --git a/lib/Parse/ParseOpenMP.cpp b/lib/Parse/ParseOpenMP.cpp
index 9871e5bbf9fafe31dc0ac346bb92ae9c692e9abd..cdcbf797678bd50b6592ffefe7dfb21d7ec36622 100644 (file)
--- a/lib/Parse/ParseOpenMP.cpp
+++ b/lib/Parse/ParseOpenMP.cpp
@@ -353,8 +353,14 @@ Parser::ParseOpenMPDeclareReductionDirective(AccessSpecifier AS) {
         // Check if initializer is omp_priv <init_expr> or something else.
         if (Tok.is(tok::identifier) &&
             Tok.getIdentifierInfo()->isStr("omp_priv")) {
-          ConsumeToken();
-          ParseOpenMPReductionInitializerForDecl(OmpPrivParm);
+          if (Actions.getLangOpts().CPlusPlus) {
+            InitializerResult = Actions.ActOnFinishFullExpr(
+                ParseAssignmentExpression().get(), D->getLocation(),
+                /*DiscardedValue=*/true);
+          } else {
+            ConsumeToken();
+            ParseOpenMPReductionInitializerForDecl(OmpPrivParm);
+          }
         } else {
           InitializerResult = Actions.ActOnFinishFullExpr(
               ParseAssignmentExpression().get(), D->getLocation(),

diff --git a/lib/Sema/SemaOpenMP.cpp b/lib/Sema/SemaOpenMP.cpp
index 7287194683daa3b1393b7161f3ba1cd16f40bc35..ffcb4ba2de5ef38c777144aafbf4cb2a19c1cba0 100644 (file)
--- a/lib/Sema/SemaOpenMP.cpp
+++ b/lib/Sema/SemaOpenMP.cpp
@@ -12763,6 +12763,11 @@ void Sema::ActOnOpenMPDeclareReductionCombinerStart(Scope *S, Decl *D) {
     DRD->addDecl(OmpInParm);
     DRD->addDecl(OmpOutParm);
   }
+  Expr *InE =
+      ::buildDeclRefExpr(*this, OmpInParm, ReductionType, D->getLocation());
+  Expr *OutE =
+      ::buildDeclRefExpr(*this, OmpOutParm, ReductionType, D->getLocation());
+  DRD->setCombinerData(InE, OutE);
 }
 
 void Sema::ActOnOpenMPDeclareReductionCombinerEnd(Decl *D, Expr *Combiner) {
@@ -12818,6 +12823,11 @@ VarDecl *Sema::ActOnOpenMPDeclareReductionInitializerStart(Scope *S, Decl *D) {
     DRD->addDecl(OmpPrivParm);
     DRD->addDecl(OmpOrigParm);
   }
+  Expr *OrigE =
+      ::buildDeclRefExpr(*this, OmpOrigParm, ReductionType, D->getLocation());
+  Expr *PrivE =
+      ::buildDeclRefExpr(*this, OmpPrivParm, ReductionType, D->getLocation());
+  DRD->setInitializerData(OrigE, PrivE);
   return OmpPrivParm;
 }
 

diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 2c29b8cbcc197c58982f5ba7c9ad1c5a453d1742..09aa25ca0a28916e76f45d9bc4bdc059f8d01711 100644 (file)
--- a/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2733,10 +2733,19 @@ Decl *TemplateDeclInstantiator::VisitOMPThreadPrivateDecl(
 Decl *TemplateDeclInstantiator::VisitOMPDeclareReductionDecl(
     OMPDeclareReductionDecl *D) {
   // Instantiate type and check if it is allowed.
-  QualType SubstReductionType = SemaRef.ActOnOpenMPDeclareReductionType(
-      D->getLocation(),
-      ParsedType::make(SemaRef.SubstType(D->getType(), TemplateArgs,
-                                         D->getLocation(), DeclarationName())));
+  const bool RequiresInstantiation =
+      D->getType()->isDependentType() ||
+      D->getType()->isInstantiationDependentType() ||
+      D->getType()->containsUnexpandedParameterPack();
+  QualType SubstReductionType;
+  if (RequiresInstantiation) {
+    SubstReductionType = SemaRef.ActOnOpenMPDeclareReductionType(
+        D->getLocation(),
+        ParsedType::make(SemaRef.SubstType(
+            D->getType(), TemplateArgs, D->getLocation(), DeclarationName())));
+  } else {
+    SubstReductionType = D->getType();
+  }
   if (SubstReductionType.isNull())
     return nullptr;
   bool IsCorrect = !SubstReductionType.isNull();
@@ -2753,25 +2762,35 @@ Decl *TemplateDeclInstantiator::VisitOMPDeclareReductionDecl(
       /*S=*/nullptr, Owner, D->getDeclName(), ReductionTypes, D->getAccess(),
       PrevDeclInScope);
   auto *NewDRD = cast<OMPDeclareReductionDecl>(DRD.get().getSingleDecl());
-  if (isDeclWithinFunction(NewDRD))
-    SemaRef.CurrentInstantiationScope->InstantiatedLocal(D, NewDRD);
+  SemaRef.CurrentInstantiationScope->InstantiatedLocal(D, NewDRD);
+  if (!RequiresInstantiation) {
+    if (Expr *Combiner = D->getCombiner()) {
+      NewDRD->setCombinerData(D->getCombinerIn(), D->getCombinerOut());
+      NewDRD->setCombiner(Combiner);
+      if (Expr *Init = D->getInitializer()) {
+        NewDRD->setInitializerData(D->getInitOrig(), D->getInitPriv());
+        NewDRD->setInitializer(Init, D->getInitializerKind());
+      }
+    }
+    (void)SemaRef.ActOnOpenMPDeclareReductionDirectiveEnd(
+        /*S=*/nullptr, DRD, IsCorrect && !D->isInvalidDecl());
+    return NewDRD;
+  }
   Expr *SubstCombiner = nullptr;
   Expr *SubstInitializer = nullptr;
   // Combiners instantiation sequence.
   if (D->getCombiner()) {
     SemaRef.ActOnOpenMPDeclareReductionCombinerStart(
         /*S=*/nullptr, NewDRD);
-    const char *Names[] = {"omp_in", "omp_out"};
-    for (auto &Name : Names) {
-      DeclarationName DN(&SemaRef.Context.Idents.get(Name));
-      auto OldLookup = D->lookup(DN);
-      auto Lookup = NewDRD->lookup(DN);
-      if (!OldLookup.empty() && !Lookup.empty()) {
-        assert(Lookup.size() == 1 && OldLookup.size() == 1);
-        SemaRef.CurrentInstantiationScope->InstantiatedLocal(OldLookup.front(),
-                                                             Lookup.front());
-      }
-    }
+    SemaRef.CurrentInstantiationScope->InstantiatedLocal(
+        cast<DeclRefExpr>(D->getCombinerIn())->getDecl(),
+        cast<DeclRefExpr>(NewDRD->getCombinerIn())->getDecl());
+    SemaRef.CurrentInstantiationScope->InstantiatedLocal(
+        cast<DeclRefExpr>(D->getCombinerOut())->getDecl(),
+        cast<DeclRefExpr>(NewDRD->getCombinerOut())->getDecl());
+    auto *ThisContext = dyn_cast_or_null<CXXRecordDecl>(Owner);
+    Sema::CXXThisScopeRAII ThisScope(SemaRef, ThisContext, /*TypeQuals*/ 0,
+                                     ThisContext);
     SubstCombiner = SemaRef.SubstExpr(D->getCombiner(), TemplateArgs).get();
     SemaRef.ActOnOpenMPDeclareReductionCombinerEnd(NewDRD, SubstCombiner);
     // Initializers instantiation sequence.
@@ -2779,19 +2798,12 @@ Decl *TemplateDeclInstantiator::VisitOMPDeclareReductionDecl(
       VarDecl *OmpPrivParm =
           SemaRef.ActOnOpenMPDeclareReductionInitializerStart(
               /*S=*/nullptr, NewDRD);
-      const char *Names[] = {"omp_orig", "omp_priv"};
-      for (auto &Name : Names) {
-        DeclarationName DN(&SemaRef.Context.Idents.get(Name));
-        auto OldLookup = D->lookup(DN);
-        auto Lookup = NewDRD->lookup(DN);
-        if (!OldLookup.empty() && !Lookup.empty()) {
-          assert(Lookup.size() == 1 && OldLookup.size() == 1);
-          auto *OldVD = cast<VarDecl>(OldLookup.front());
-          auto *NewVD = cast<VarDecl>(Lookup.front());
-          SemaRef.InstantiateVariableInitializer(NewVD, OldVD, TemplateArgs);
-          SemaRef.CurrentInstantiationScope->InstantiatedLocal(OldVD, NewVD);
-        }
-      }
+      SemaRef.CurrentInstantiationScope->InstantiatedLocal(
+          cast<DeclRefExpr>(D->getInitOrig())->getDecl(),
+          cast<DeclRefExpr>(NewDRD->getInitOrig())->getDecl());
+      SemaRef.CurrentInstantiationScope->InstantiatedLocal(
+          cast<DeclRefExpr>(D->getInitPriv())->getDecl(),
+          cast<DeclRefExpr>(NewDRD->getInitPriv())->getDecl());
       if (D->getInitializerKind() == OMPDeclareReductionDecl::CallInit) {
         SubstInitializer =
             SemaRef.SubstExpr(D->getInitializer(), TemplateArgs).get();
@@ -2808,8 +2820,9 @@ Decl *TemplateDeclInstantiator::VisitOMPDeclareReductionDecl(
           SubstInitializer) ||
          (D->getInitializerKind() != OMPDeclareReductionDecl::CallInit &&
           !SubstInitializer && !SubstInitializer));
-  } else
+  } else {
     IsCorrect = false;
+  }
 
   (void)SemaRef.ActOnOpenMPDeclareReductionDirectiveEnd(/*S=*/nullptr, DRD,
                                                         IsCorrect);
@@ -4897,7 +4910,9 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc, NamedDecl *D,
     return D;
   if (isa<ParmVarDecl>(D) || isa<NonTypeTemplateParmDecl>(D) ||
       isa<TemplateTypeParmDecl>(D) || isa<TemplateTemplateParmDecl>(D) ||
-      (ParentDC->isFunctionOrMethod() && ParentDC->isDependentContext()) ||
+      ((ParentDC->isFunctionOrMethod() ||
+        isa<OMPDeclareReductionDecl>(ParentDC)) &&
+       ParentDC->isDependentContext()) ||
       (isa<CXXRecordDecl>(D) && cast<CXXRecordDecl>(D)->isLambda())) {
     // D is a local of some kind. Look into the map of local
     // declarations to their instantiations.

diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp
index 829510747647d9676fd8f9c04357f89ca3c4fcba..d06a6226af7859b0ed8d16915fa3421da2155bf4 100644 (file)
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/lib/Serialization/ASTReaderDecl.cpp
@@ -2633,10 +2633,17 @@ void ASTDeclReader::VisitOMPThreadPrivateDecl(OMPThreadPrivateDecl *D) {
 void ASTDeclReader::VisitOMPDeclareReductionDecl(OMPDeclareReductionDecl *D) {
   VisitValueDecl(D);
   D->setLocation(ReadSourceLocation());
-  D->setCombiner(Record.readExpr());
-  D->setInitializer(
-      Record.readExpr(),
-      static_cast<OMPDeclareReductionDecl::InitKind>(Record.readInt()));
+  Expr *In = Record.readExpr();
+  Expr *Out = Record.readExpr();
+  D->setCombinerData(In, Out);
+  Expr *Combiner = Record.readExpr();
+  D->setCombiner(Combiner);
+  Expr *Orig = Record.readExpr();
+  Expr *Priv = Record.readExpr();
+  D->setInitializerData(Orig, Priv);
+  Expr *Init = Record.readExpr();
+  auto IK = static_cast<OMPDeclareReductionDecl::InitKind>(Record.readInt());
+  D->setInitializer(Init, IK);
   D->PrevDeclInScope = ReadDeclID();
 }
 

diff --git a/lib/Serialization/ASTWriterDecl.cpp b/lib/Serialization/ASTWriterDecl.cpp
index 8eb6c61525dcc08c005b178e3583db8467202b61..d757f609ea2e8b8b45afe2ecfb021abdb84fef16 100644 (file)
--- a/lib/Serialization/ASTWriterDecl.cpp
+++ b/lib/Serialization/ASTWriterDecl.cpp
@@ -1742,7 +1742,11 @@ void ASTDeclWriter::VisitOMPThreadPrivateDecl(OMPThreadPrivateDecl *D) {
 void ASTDeclWriter::VisitOMPDeclareReductionDecl(OMPDeclareReductionDecl *D) {
   VisitValueDecl(D);
   Record.AddSourceLocation(D->getBeginLoc());
+  Record.AddStmt(D->getCombinerIn());
+  Record.AddStmt(D->getCombinerOut());
   Record.AddStmt(D->getCombiner());
+  Record.AddStmt(D->getInitOrig());
+  Record.AddStmt(D->getInitPriv());
   Record.AddStmt(D->getInitializer());
   Record.push_back(D->getInitializerKind());
   Record.AddDeclRef(D->getPrevDeclInScope());

diff --git a/test/OpenMP/declare_reduction_messages.cpp b/test/OpenMP/declare_reduction_messages.cpp
index f22f92485a57e103f4dbb034c23387fa37a59b5e..21c03fac228ded2576b8596d37850f0781e29d74 100644 (file)
--- a/test/OpenMP/declare_reduction_messages.cpp
+++ b/test/OpenMP/declare_reduction_messages.cpp
@@ -142,17 +142,33 @@ int main() {
 #if __cplusplus == 201103L
 struct A {
   A() {}
-  // expected-note@+1 {{copy constructor is implicitly deleted because 'A' has a user-declared move assignment operator}}
   A& operator=(A&&) = default;
 };
 
 int A_TEST() {
   A test;
-// expected-error@+1 {{call to implicitly-deleted copy constructor of 'A'}}
 #pragma omp declare reduction(+ : A : omp_out) initializer(omp_priv = A())
-// expected-error@+1 {{invalid operands to binary expression ('A' and 'A')}}
 #pragma omp parallel reduction(+ : test)
   {}
   return 0;
 }
+
+struct U
+{
+  void foo(U&, bool);
+  U();
+};
+template <int N>
+struct S
+{
+  int s;
+  // expected-note@+1 {{'foo' declared here}}
+  void foo(S &x) {};
+  // expected-error@+1 {{too many arguments to function call, expected single argument 'x', have 2 arguments}}
+  #pragma omp declare reduction (foo : U, S : omp_out.foo(omp_in, false))
+};
+// expected-warning@+2 {{extra tokens at the end of '#pragma omp declare reduction' are ignored}}
+// expected-note@+1 {{in instantiation of template class 'S<1>' requested here}}
+#pragma omp declare reduction (bar : S<1> : omp_out.foo(omp_in))
+
 #endif