[OPENMP] Eliminate extra set of simd variant function attribute.

[jyu2]

Summary

Current clang generates extra set of simd variant function attribute
with extra 'v' encoding.
For example:
_ZGVbN2vZ5add_1Pf vs _ZGVbN2vvZ5add_1Pf
The problem is due to declaration of ParamAttrs following:

llvm::SmallVector<ParamAttrTy, 8> ParamAttrs(ParamPositions.size());

where ParamPositions.size() is grown after following assignment:

Pos = ParamPositions[PVD];

So the PVD is not find in ParamPositions.

The problem is ParamPositions need to set for each FD decl. To fix this

Move ParamPositions's init inside while loop for each FD.

[ABataev]

If ParamPositions grows, it means that the item is not found. Is it correct behavior if the compiler inserts new item in the ParamPositions map?

[jyu2]

In D122338#3403414, @ABataev wrote:

If ParamPositions grows, it means that the item is not found. Is it correct behavior if the compiler inserts new item in the ParamPositions map?

I see. Thanks! Then it seems ParamPositions need to be set for each function decl otherwise PVD can not find.

[ABataev]

clang/lib/CodeGen/CGOpenMPRuntime.cpp

11959

I would also recommend to replace all operator [] calls to something that does not modify the map, like find.

[jyu2]

Address Alexey's comment.

[ABataev]

clang/lib/CodeGen/CGOpenMPRuntime.cpp

11963

Better to use count() or contain() (do not remember which one is defined for DenseMap), also add messages to asserts.

[ABataev]

clang/lib/CodeGen/CGOpenMPRuntime.cpp

11963–11964

Or, if you already using find, better to do something like:

auto It = ParamPositions.find(PVD);
assert(It != ParamPositions.end() && "...");
Pos = It->second;

[jyu2]

Address Alexey comment.

[ABataev]

LG

[jyu2]

In D122338#3405948, @ABataev wrote:

LG

Thank you so much Alexey.

clang/lib/CodeGen/CGOpenMPRuntime.cpp

11959

How about adding assert? Thanks.

clang/lib/CodeGen/CGOpenMPRuntime.cpp

if (UserVLEN) {
     }
   }
 }
 
 void CGOpenMPRuntime::emitDeclareSimdFunction(const FunctionDecl *FD,
                                               llvm::Function *Fn) {
   ASTContext &C = CGM.getContext();
   FD = FD->getMostRecentDecl();
+  while (FD) {
     // Map params to their positions in function decl.
     llvm::DenseMap<const Decl *, unsigned> ParamPositions;
     if (isa<CXXMethodDecl>(FD))
       ParamPositions.try_emplace(FD, 0);
     unsigned ParamPos = ParamPositions.size();
     for (const ParmVarDecl *P : FD->parameters()) {
       ParamPositions.try_emplace(P->getCanonicalDecl(), ParamPos);
       ++ParamPos;
     }
-  while (FD) {
     for (const auto *Attr : FD->specific_attrs<OMPDeclareSimdDeclAttr>()) {
       llvm::SmallVector<ParamAttrTy, 8> ParamAttrs(ParamPositions.size());
       // Mark uniform parameters.
       for (const Expr *E : Attr->uniforms()) {
         E = E->IgnoreParenImpCasts();
         unsigned Pos;
         if (isa<CXXThisExpr>(E)) {
           Pos = ParamPositions[FD];
         } else {
           const auto *PVD = cast<ParmVarDecl>(cast<DeclRefExpr>(E)->getDecl())
                                 ->getCanonicalDecl();
-          Pos = ParamPositions[PVD];
+          auto It = ParamPositions.find(PVD);
+          assert(It != ParamPositions.end() && "Function parameter not found");
+          Pos = It->second;
         }
         ParamAttrs[Pos].Kind = Uniform;
       }
       // Get alignment info.
       auto *NI = Attr->alignments_begin();
       for (const Expr *E : Attr->aligneds()) {
         E = E->IgnoreParenImpCasts();
         unsigned Pos;
         QualType ParmTy;
         if (isa<CXXThisExpr>(E)) {
           Pos = ParamPositions[FD];
           ParmTy = E->getType();
         } else {
           const auto *PVD = cast<ParmVarDecl>(cast<DeclRefExpr>(E)->getDecl())
                                 ->getCanonicalDecl();
-          Pos = ParamPositions[PVD];
+          auto It = ParamPositions.find(PVD);
+          assert(It != ParamPositions.end() && "Function parameter not found");
+          Pos = It->second;
           ParmTy = PVD->getType();
         }
         ParamAttrs[Pos].Alignment =
             (*NI)
                 ? (*NI)->EvaluateKnownConstInt(C)
                 : llvm::APSInt::getUnsigned(
                       C.toCharUnitsFromBits(C.getOpenMPDefaultSimdAlign(ParmTy))
                           .getQuantity());
         ++NI;
       }
       // Mark linear parameters.
       auto *SI = Attr->steps_begin();
       for (const Expr *E : Attr->linears()) {
         E = E->IgnoreParenImpCasts();
         unsigned Pos;
         // Rescaling factor needed to compute the linear parameter
         // value in the mangled name.
         unsigned PtrRescalingFactor = 1;
         if (isa<CXXThisExpr>(E)) {
           Pos = ParamPositions[FD];
         } else {
           const auto *PVD = cast<ParmVarDecl>(cast<DeclRefExpr>(E)->getDecl())
                                 ->getCanonicalDecl();
-          Pos = ParamPositions[PVD];
+          auto It = ParamPositions.find(PVD);
+          assert(It != ParamPositions.end() && "Function parameter not found");
+          Pos = It->second;
           if (auto *P = dyn_cast<PointerType>(PVD->getType()))
             PtrRescalingFactor = CGM.getContext()
                                      .getTypeSizeInChars(P->getPointeeType())
                                      .getQuantity();
         }
         ParamAttrTy &ParamAttr = ParamAttrs[Pos];
         ParamAttr.Kind = Linear;
         // Assuming a stride of 1, for `linear` without modifiers.
         ParamAttr.StrideOrArg = llvm::APSInt::getUnsigned(1);
         if (*SI) {
           Expr::EvalResult Result;
           if (!(*SI)->EvaluateAsInt(Result, C, Expr::SE_AllowSideEffects)) {
             if (const auto *DRE =
                     cast<DeclRefExpr>((*SI)->IgnoreParenImpCasts())) {
               if (const auto *StridePVD =
                       dyn_cast<ParmVarDecl>(DRE->getDecl())) {
                 ParamAttr.Kind = LinearWithVarStride;
-                ParamAttr.StrideOrArg = llvm::APSInt::getUnsigned(
-                    ParamPositions[StridePVD->getCanonicalDecl()]);
+                auto It = ParamPositions.find(StridePVD->getCanonicalDecl());
+                assert(It != ParamPositions.end() &&
+                       "Function parameter not found");
+                ParamAttr.StrideOrArg = llvm::APSInt::getUnsigned(It->second);
               }
             }
           } else {
             ParamAttr.StrideOrArg = Result.Val.getInt();
           }
         }
         // If we are using a linear clause on a pointer, we need to
         // rescale the value of linear_step with the byte size of the

clang/test/OpenMP/declare_simd_codegen.cpp

 // CHECK-DAG: "_ZGVcM32v__Z5add_1Pf"
 // CHECK-DAG: "_ZGVdM32v__Z5add_1Pf"
 // CHECK-DAG: "_ZGVeM32v__Z5add_1Pf"
 // CHECK-DAG: "_ZGVbN2v__Z5add_1Pf"
 // CHECK-DAG: "_ZGVcN4v__Z5add_1Pf"
 // CHECK-DAG: "_ZGVdN4v__Z5add_1Pf"
 // CHECK-DAG: "_ZGVeN8v__Z5add_1Pf"
 
+// CHECK-NOT: _ZGVbN2vv__Z5add_1Pf
+// CHECK-NOT: _ZGVcN4vv__Z5add_1Pf
+// CHECK-NOT: _ZGVdN4vv__Z5add_1Pf
+// CHECK-NOT: _ZGVeN8vv__Z5add_1Pf
+// CHECK-NOT: _ZGVbM32vv__Z5add_1Pf
+// CHECK-NOT: _ZGVcM32vv__Z5add_1Pf
+// CHECK-NOT: _ZGVdM32vv__Z5add_1Pf
+// CHECK-NOT: _ZGVeM32vv__Z5add_1Pf
+// CHECK-NOT: _ZGVbN4l32v__Z5add_1Pf
+// CHECK-NOT: _ZGVcN8l32v__Z5add_1Pf
+// CHECK-NOT: _ZGVdN8l32v__Z5add_1Pf
+// CHECK-NOT: _ZGVeN16l32v__Z5add_1Pf
+// CHECK-NOT: _ZGVbM4l32v__Z5add_1Pf
+// CHECK-NOT: _ZGVcM8l32v__Z5add_1Pf
+// CHECK-NOT: _ZGVdM8l32v__Z5add_1Pf
+// CHECK-NOT: _ZGVeM16l32v__Z5add_1Pf
+
 // CHECK-DAG: "_ZGVbM2va16va16vv__Z1hIiEvPT_S1_S1_S1_"
 // CHECK-DAG: "_ZGVbN2va16va16vv__Z1hIiEvPT_S1_S1_S1_"
 // CHECK-DAG: "_ZGVcM4va16va16vv__Z1hIiEvPT_S1_S1_S1_"
 // CHECK-DAG: "_ZGVcN4va16va16vv__Z1hIiEvPT_S1_S1_S1_"
 // CHECK-DAG: "_ZGVdM4va16va16vv__Z1hIiEvPT_S1_S1_S1_"
 // CHECK-DAG: "_ZGVdN4va16va16vv__Z1hIiEvPT_S1_S1_S1_"
 // CHECK-DAG: "_ZGVeM8va16va16vv__Z1hIiEvPT_S1_S1_S1_"
 // CHECK-DAG: "_ZGVeN8va16va16vv__Z1hIiEvPT_S1_S1_S1_"