[OMPIRBuilder] Add support for simdlen clause

[psoni2628]

Summary

This patch adds OMPIRBuilder support for the simdlen clause for the
simd directive. It uses the simdlen support in OpenMPIRBuilder when
it is enabled in Clang. Simdlen is lowered by OpenMPIRBuilder by
generating the loop.vectorize.width metadata.

[arnamoy10]

clang/lib/CodeGen/CGStmtOpenMP.cpp

2596–2599

I am just wondering whether we should have a check to make sure that we are processing the clauses of only simd directive here. Because the function takes a general OMPExecutableDirective as argument

[psoni2628]

clang/lib/CodeGen/CGStmtOpenMP.cpp

2596–2599

That's a fair point. I guess isSupportedByOpenMPIRBuilder could be used for other directive types other than simd, even though it's not right now.

[psoni2628]

clang/lib/CodeGen/CGStmtOpenMP.cpp

2596–2599

Would it make more sense to only guard the checking of clauses with a check for OMPSimdDirective, or the whole thing? I believe even the code below, which checks for an ordered directive, is also specifically for simd?

Example of guarding the whole thing:

if(dyn_cast<OMPSimdDirective>(S)) {
  // Check for unsupported clauses
  for (OMPClause *C : S.clauses()) {
    // Currently only simdlen clause is supported
    if (dyn_cast<OMPSimdlenClause>(C))
      continue;
    else
      return false;
  }

  // Check if we have a statement with the ordered directive.
  // Visit the statement hierarchy to find a compound statement
  // with a ordered directive in it.
  if (const auto *CanonLoop = dyn_cast<OMPCanonicalLoop>(S.getRawStmt())) {
    if (const Stmt *SyntacticalLoop = CanonLoop->getLoopStmt()) {
      for (const Stmt *SubStmt : SyntacticalLoop->children()) {
        if (!SubStmt)
          continue;
        if (const CompoundStmt *CS = dyn_cast<CompoundStmt>(SubStmt)) {
          for (const Stmt *CSSubStmt : CS->children()) {
            if (!CSSubStmt)
              continue;
            if (isa<OMPOrderedDirective>(CSSubStmt)) {
              return false;
            }
          }
        }
      }
    }
  }
}

[domada]

clang/test/OpenMP/irbuilder_simd.cpp

15

Could you add separate test case instead of modifying existing test case?

[shraiysh]

clang/lib/CodeGen/CGStmtOpenMP.cpp

2596–2599

Can we instead have separate isSupportedByOpenMPIRBuilder for every directive to avoid bloating the function with checks and if conditions?

static bool isSupportedByOpenMPIRBuilder(const OMPSimdDirective &S) {...}
void CodeGenFunction::EmitOMPSimdDirective(const OMPSimdDirective &S) {...}

static bool isSupportedByOpenMPIRBuilder(const OMPOrderedDirective &S) {...}
void CodeGenFunction::EmitOMPOrderedDirective(const OMPOrderedDirective &S) {...}

static bool isSupportedByOpenMPIRBuilder(const OMPTaskDirective &S) {...}
void CodeGenFunction::EmitOMPOrderedDirective(const OMPTaskDirective &S) {...}

clang/test/OpenMP/irbuilder_simd.cpp

74

nit: maybe add newline

llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

1773

Can we please add this as a new test instead of modifying the existing one?

[psoni2628]

clang/lib/CodeGen/CGStmtOpenMP.cpp

2596–2599

It was decided in D114379 to use OMPExecutableDirective in order to allow this function to be reused for constructs such as for simd. Do you wish to undo this now, and instead specialize the function?

[shraiysh]

clang/lib/CodeGen/CGStmtOpenMP.cpp

2596–2599

I wasn't aware of the earlier discussion about this. However it seems like it was suggested to try either splitting the function or merging it with slight preference for merging (I didn't understand how that would help constructs like for simd because I haven't worked with for simd much and will take others' word for it).

The suggested code change in previous reply - checking of clauses with a check for OMPSimdDirective - this seems like it would eventually become a huge function because of such checks.

@Meinersbur it would be great if you could please advise on this - if keeping the checks for all executable directives in the same function will be helpful in the long run - allowing us to reuse checks or should we split it? A third alternative would be to have both functions, one specialized for SIMD and one for ExecutableConstruct, where the latter might handle combined constructs like for simd etc. Would that arrangement work?

[psoni2628]

• Create new tests instead of modifying existing ones
• Specialize isSupportedByOpenMPIRBuilder for OMPSimdDirective

clang/lib/CodeGen/CGStmtOpenMP.cpp

2596–2599

I agree with you that having a general function for OMPExecutableDirective will probably result in a very large, bloated function. I have tentatively just specialized the function for OMPSimdDirective.

[kiranchandramohan]

Nit: Also add to the summary that this patch uses the simdlen support in OpenMPIRBuilder when it is enabled in Clang.

clang/lib/CodeGen/CGStmtOpenMP.cpp

2600

Nit: Else after return/continue is discouraged.
https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return

[psoni2628]

In D129149#3632813, @kiranchandramohan wrote:

Nit: Also add to the summary that this patch uses the simdlen support in OpenMPIRBuilder when it is enabled in Clang.

I just wanted to clarify that below is what you mean?

This patch adds OMPIRBuilder support for the simdlen clause for the
simd directive. It uses the simdlen support in OpenMPIRBuilder when
it is enabled in Clang.

[kiranchandramohan]

In D129149#3632861, @psoni2628 wrote:

In D129149#3632813, @kiranchandramohan wrote:

Nit: Also add to the summary that this patch uses the simdlen support in OpenMPIRBuilder when it is enabled in Clang.

I just wanted to clarify that below is what you mean?

This patch adds OMPIRBuilder support for the simdlen clause for the
simd directive. It uses the simdlen support in OpenMPIRBuilder when
it is enabled in clang.

Yes. Thanks.

Maybe you can also add that simdlen is lowered by the OpenMPIRBuilder by generating the loop.vectorize.width metadata.

[psoni2628]

• Remove discouraged else after return

[jdoerfert]

Generally, this is fine. Some nits. The only reason not to accept this right now is the test. Why manual check lines?
Wrt. the function signature. As soon as we have more then SIMD directives to check we refactor things. Keep it simple
until you need the functionality.

clang/lib/CodeGen/CGStmtOpenMP.cpp

2598

clang/test/OpenMP/irbuilder_simdlen.cpp

2

The check lines look auto-generated and then modified by hand. Why is that?

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

608

No debug location needed. You also copied the comment that makes little sene.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

2923

Also above, no llvm::

[psoni2628]

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

2923

It isn't used in the original applySimd either. Should I remove it in both places?

[psoni2628]

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

608

It isn't used in the original applySimd either. Should I remove it in both places?

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

2923

Wrong comment to reply to. Please ignore this.

[jdoerfert]

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

608

Yes.

[psoni2628]

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

608

Looks like I'll need to fix applySimd on the MLIR side too. I think it makes more sense to do that separately in a different patch. For now, I will leave applySimd with the unused DebugLocation. I will fix applySimdlen.

[psoni2628]

• Autogenerate check lines for test case
• Use isa instead of dyncast
• Remove unused DebugLoc
• Remove llvm:: from llvm::ConstantInt

[psoni2628]

clang/test/OpenMP/irbuilder_simdlen.cpp

2

I originally took irbuilder_simd.cpp and modified it to include simdlen. I have now auto-generated the check lines.

[jdoerfert]

LG, let's update the test script first though. See below.

clang/test/OpenMP/irbuilder_simdlen.cpp

2

I see. Auto generated is great, but you don't actually check the metadata anymore. Add --check-globals to the update script command.
You might also want to teach the script about access group metadata, see D101742 for an example how to do this.

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

610

As mentioned before. No llvm::. This is not MLIR.

[Meinersbur]

Why not have simdlen an argument to applySimd (which can be null if not used)? In this case it happens to be just adding some metadata, but for others such as unrollLoopPartial it does not make sense to have the unroll factor set by a different method. It would also allow better consistency checks if passed as a parameter.

clang/lib/CodeGen/CGStmtOpenMP.cpp

2596–2599

A function taking only OMPSimdDirective will still not handle OMPForSimdDirective and the code for handling it would need to be duplicated for both (and OMPDistributeSimdDirective, and OMPDistributeParallelForSimdDirective, ...)

I'd expect the check the most common to be the (non-)existence of a clause such as simdlen. Once support for it is added, it should be supported for all combined/composite constructs. Code Duplication not necessary. We do not need to check whether OMPExecutableDirective is is a simd construct because Sema will reject simdlen on any other construct anyway.

Therefore is still favor taking a OMPExecutableDirective argument. OpenMPIRBuilder does not support combined/composite construct yet, but whoever adds it has to face the immense problem of refactoring all the isSupportedByOpenMPIRBuilder implementations to have to rewrite that function for all combined/composite constructs. Better consider the possibility of combined/composite constructs from the beginning.

[jdoerfert]

In D129149#3633627, @Meinersbur wrote:

Why not have simdlen an argument to applySimd (which can be null if not used)? In this case it happens to be just adding some metadata, but for others such as unrollLoopPartial it does not make sense to have the unroll factor set by a different method. It would also allow better consistency checks if passed as a parameter.

Sure.

clang/lib/CodeGen/CGStmtOpenMP.cpp

2596–2599

Unsure why this has to go into another round of comments:

This is, as I noted, a mood discussion. If this passes all tests there is no good reason to make it OMPExecutableDirective right now. As we add anything that does not work with this API we need to make a choice and include changes anyway. Arguing the type of the argument right now, = the ~15 letter difference, is making anything harder in the future, is arguably not true. As always, designing something without coverage and use case at hand is discouraged, especially if the design does not exclude an extension.

[psoni2628]

• Remove applySimdlen and apply simdlen in applySimd
• Remove unnecessary DebugLocation from applySimd in both OpenMPIRBuilder and Flang MLIR
• Add llvm.access.group to llvm/utils/UpdateTestChecks/common.py
• Fix test case by adding --check-globals

[Meinersbur]

LGTM

[psoni2628]

• Rebase, ensuring that build bot failures are gone

clang/lib/CodeGen/CGStmtOpenMP.cpp

static void emitOMPSimdRegion(CodeGenFunction &CGF, const OMPLoopDirective &S,
   CGF.EmitOMPLinearClauseFinal(S, [](CodeGenFunction &) { return nullptr; });
   // Emit: if (PreCond) - end.
   if (ContBlock) {
     CGF.EmitBranch(ContBlock);
     CGF.EmitBlock(ContBlock, true);
   }
 }
 
-static bool isSupportedByOpenMPIRBuilder(const OMPExecutableDirective &S) {
+static bool isSupportedByOpenMPIRBuilder(const OMPSimdDirective &S) {
   // Check for unsupported clauses
-  if (!S.clauses().empty()) {
-    // Currently no clause is supported
+  for (OMPClause *C : S.clauses()) {
+    // Currently only simdlen clause is supported
+    if (!isa<OMPSimdlenClause>(C))
       return false;
   }
 
   // Check if we have a statement with the ordered directive.
   // Visit the statement hierarchy to find a compound statement
   // with a ordered directive in it.
   if (const auto *CanonLoop = dyn_cast<OMPCanonicalLoop>(S.getRawStmt())) {
     if (const Stmt *SyntacticalLoop = CanonLoop->getLoopStmt()) {
       for (const Stmt *SubStmt : SyntacticalLoop->children()) {
         if (!SubStmt)
void CodeGenFunction::EmitOMPSimdDirective(const OMPSimdDirective &S) {
   bool UseOMPIRBuilder =
       CGM.getLangOpts().OpenMPIRBuilder && isSupportedByOpenMPIRBuilder(S);
   if (UseOMPIRBuilder) {
     auto &&CodeGenIRBuilder = [this, &S, UseOMPIRBuilder](CodeGenFunction &CGF,
                                                           PrePostActionTy &) {
       // Use the OpenMPIRBuilder if enabled.
       if (UseOMPIRBuilder) {
         // Emit the associated statement and get its loop representation.
-        llvm::DebugLoc DL = SourceLocToDebugLoc(S.getBeginLoc());
         const Stmt *Inner = S.getRawStmt();
         llvm::CanonicalLoopInfo *CLI =
             EmitOMPCollapsedCanonicalLoopNest(Inner, 1);
 
         llvm::OpenMPIRBuilder &OMPBuilder =
             CGM.getOpenMPRuntime().getOMPBuilder();
         // Add SIMD specific metadata
-        OMPBuilder.applySimd(DL, CLI);
+        llvm::ConstantInt *Simdlen = nullptr;
+        if (const auto *C = S.getSingleClause<OMPSimdlenClause>()) {
+          RValue Len =
+              this->EmitAnyExpr(C->getSimdlen(), AggValueSlot::ignored(),
+                                /*ignoreResult=*/true);
+          auto *Val = cast<llvm::ConstantInt>(Len.getScalarVal());
+          Simdlen = Val;
+        }
+        OMPBuilder.applySimd(CLI, Simdlen);
         return;
       }
     };
     {
       auto LPCRegion =
           CGOpenMPRuntime::LastprivateConditionalRAII::disable(*this, S);
       OMPLexicalScope Scope(*this, S, OMPD_unknown);
       CGM.getOpenMPRuntime().emitInlinedDirective(*this, OMPD_simd,

clang/test/OpenMP/irbuilder_simd.cpp

 // RUN: %clang_cc1 -no-opaque-pointers -fopenmp-enable-irbuilder -verify -fopenmp -fopenmp-version=45 -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s 
 // expected-no-diagnostics
 
 struct S {
   int a, b;
 };
 
 struct P {
   int a, b;
 };
 
 void simple(float *a, float *b, int *c) {
   S s, *p;
   P pp;
 #pragma omp simd
   for (int i = 3; i < 32; i += 5) {
     // llvm.access.group test
     // CHECK: %[[A_ADDR:.+]] = alloca float*, align 8
     // CHECK: %[[B_ADDR:.+]] = alloca float*, align 8
     // CHECK: %[[S:.+]] = alloca %struct.S, align 4
     // CHECK: %[[P:.+]] = alloca %struct.S*, align 8
     // CHECK: %[[I:.+]] = alloca i32, align 4
     // CHECK: %[[TMP3:.+]] = load float*, float** %[[B_ADDR:.+]], align 8, !llvm.access.group ![[META3:[0-9]+]]

 }
 
 // CHECK: ![[META3:[0-9]+]] = distinct !{}
 // CHECK-NEXT: ![[META4]]  = distinct !{![[META4]], ![[META5:[0-9]+]], ![[META6:[0-9]+]]}
 // CHECK-NEXT: ![[META5]]  = !{!"llvm.loop.parallel_accesses", ![[META3]]}
 // CHECK-NEXT: ![[META6]]  = !{!"llvm.loop.vectorize.enable", i1 true}
 // CHECK-NEXT: ![[META7:[0-9]+]] = distinct !{}
 // CHECK-NEXT: ![[META8]]  = distinct !{![[META8]], ![[META9:[0-9]+]], ![[META6]]}
-// CHECK-NEXT: ![[META9]]  = !{!"llvm.loop.parallel_accesses", ![[META7]]}
+// CHECK-NEXT: ![[META9]]  = !{!"llvm.loop.parallel_accesses", ![[META7]]}
 

clang/test/OpenMP/irbuilder_simdlen.cpp

+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --check-globals
+// RUN: %clang_cc1 -no-opaque-pointers -fopenmp-enable-irbuilder -verify -fopenmp -fopenmp-version=45 -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// expected-no-diagnostics
+
+struct S {
+  int a, b;
+};
+
+struct P {
+  int a, b;
+};
+
+// CHECK-LABEL: @_Z6simplePfS_Pi(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[A_ADDR:%.*]] = alloca float*, align 8
+// CHECK-NEXT:    [[B_ADDR:%.*]] = alloca float*, align 8
+// CHECK-NEXT:    [[C_ADDR:%.*]] = alloca i32*, align 8
+// CHECK-NEXT:    [[S:%.*]] = alloca [[STRUCT_S:%.*]], align 4
+// CHECK-NEXT:    [[P:%.*]] = alloca %struct.S*, align 8
+// CHECK-NEXT:    [[PP:%.*]] = alloca [[STRUCT_P:%.*]], align 4
+// CHECK-NEXT:    [[I:%.*]] = alloca i32, align 4
+// CHECK-NEXT:    [[AGG_CAPTURED:%.*]] = alloca [[STRUCT_ANON:%.*]], align 8
+// CHECK-NEXT:    [[AGG_CAPTURED1:%.*]] = alloca [[STRUCT_ANON_0:%.*]], align 4
+// CHECK-NEXT:    [[DOTCOUNT_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:    [[J:%.*]] = alloca i32, align 4
+// CHECK-NEXT:    [[AGG_CAPTURED8:%.*]] = alloca [[STRUCT_ANON_1:%.*]], align 8
+// CHECK-NEXT:    [[AGG_CAPTURED9:%.*]] = alloca [[STRUCT_ANON_2:%.*]], align 4
+// CHECK-NEXT:    [[DOTCOUNT_ADDR10:%.*]] = alloca i32, align 4
+// CHECK-NEXT:    store float* [[A:%.*]], float** [[A_ADDR]], align 8
+// CHECK-NEXT:    store float* [[B:%.*]], float** [[B_ADDR]], align 8
+// CHECK-NEXT:    store i32* [[C:%.*]], i32** [[C_ADDR]], align 8
+// CHECK-NEXT:    store i32 3, i32* [[I]], align 4
+// CHECK-NEXT:    [[TMP0:%.*]] = getelementptr inbounds [[STRUCT_ANON]], %struct.anon* [[AGG_CAPTURED]], i32 0, i32 0
+// CHECK-NEXT:    store i32* [[I]], i32** [[TMP0]], align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = getelementptr inbounds [[STRUCT_ANON_0]], %struct.anon.0* [[AGG_CAPTURED1]], i32 0, i32 0
+// CHECK-NEXT:    [[TMP2:%.*]] = load i32, i32* [[I]], align 4
+// CHECK-NEXT:    store i32 [[TMP2]], i32* [[TMP1]], align 4
+// CHECK-NEXT:    call void @__captured_stmt(i32* [[DOTCOUNT_ADDR]], %struct.anon* [[AGG_CAPTURED]])
+// CHECK-NEXT:    [[DOTCOUNT:%.*]] = load i32, i32* [[DOTCOUNT_ADDR]], align 4
+// CHECK-NEXT:    br label [[OMP_LOOP_PREHEADER:%.*]]
+// CHECK:       omp_loop.preheader:
+// CHECK-NEXT:    br label [[OMP_LOOP_HEADER:%.*]]
+// CHECK:       omp_loop.header:
+// CHECK-NEXT:    [[OMP_LOOP_IV:%.*]] = phi i32 [ 0, [[OMP_LOOP_PREHEADER]] ], [ [[OMP_LOOP_NEXT:%.*]], [[OMP_LOOP_INC:%.*]] ]
+// CHECK-NEXT:    br label [[OMP_LOOP_COND:%.*]]
+// CHECK:       omp_loop.cond:
+// CHECK-NEXT:    [[OMP_LOOP_CMP:%.*]] = icmp ult i32 [[OMP_LOOP_IV]], [[DOTCOUNT]]
+// CHECK-NEXT:    br i1 [[OMP_LOOP_CMP]], label [[OMP_LOOP_BODY:%.*]], label [[OMP_LOOP_EXIT:%.*]]
+// CHECK:       omp_loop.body:
+// CHECK-NEXT:    call void @__captured_stmt.1(i32* [[I]], i32 [[OMP_LOOP_IV]], %struct.anon.0* [[AGG_CAPTURED1]]), !llvm.access.group [[ACC_GRP3:![0-9]+]]
+// CHECK-NEXT:    [[TMP3:%.*]] = load float*, float** [[B_ADDR]], align 8, !llvm.access.group [[ACC_GRP3]]
+// CHECK-NEXT:    [[TMP4:%.*]] = load i32, i32* [[I]], align 4, !llvm.access.group [[ACC_GRP3]]
+// CHECK-NEXT:    [[IDXPROM:%.*]] = sext i32 [[TMP4]] to i64
+// CHECK-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds float, float* [[TMP3]], i64 [[IDXPROM]]
+// CHECK-NEXT:    [[TMP5:%.*]] = load float, float* [[ARRAYIDX]], align 4, !llvm.access.group [[ACC_GRP3]]
+// CHECK-NEXT:    [[A2:%.*]] = getelementptr inbounds [[STRUCT_S]], %struct.S* [[S]], i32 0, i32 0
+// CHECK-NEXT:    [[TMP6:%.*]] = load i32, i32* [[A2]], align 4, !llvm.access.group [[ACC_GRP3]]
+// CHECK-NEXT:    [[CONV:%.*]] = sitofp i32 [[TMP6]] to float
+// CHECK-NEXT:    [[ADD:%.*]] = fadd float [[TMP5]], [[CONV]]
+// CHECK-NEXT:    [[TMP7:%.*]] = load %struct.S*, %struct.S** [[P]], align 8, !llvm.access.group [[ACC_GRP3]]
+// CHECK-NEXT:    [[A3:%.*]] = getelementptr inbounds [[STRUCT_S]], %struct.S* [[TMP7]], i32 0, i32 0
+// CHECK-NEXT:    [[TMP8:%.*]] = load i32, i32* [[A3]], align 4, !llvm.access.group [[ACC_GRP3]]
+// CHECK-NEXT:    [[CONV4:%.*]] = sitofp i32 [[TMP8]] to float
+// CHECK-NEXT:    [[ADD5:%.*]] = fadd float [[ADD]], [[CONV4]]
+// CHECK-NEXT:    [[TMP9:%.*]] = load float*, float** [[A_ADDR]], align 8, !llvm.access.group [[ACC_GRP3]]
+// CHECK-NEXT:    [[TMP10:%.*]] = load i32, i32* [[I]], align 4, !llvm.access.group [[ACC_GRP3]]
+// CHECK-NEXT:    [[IDXPROM6:%.*]] = sext i32 [[TMP10]] to i64
+// CHECK-NEXT:    [[ARRAYIDX7:%.*]] = getelementptr inbounds float, float* [[TMP9]], i64 [[IDXPROM6]]
+// CHECK-NEXT:    store float [[ADD5]], float* [[ARRAYIDX7]], align 4, !llvm.access.group [[ACC_GRP3]]
+// CHECK-NEXT:    br label [[OMP_LOOP_INC]]
+// CHECK:       omp_loop.inc:
+// CHECK-NEXT:    [[OMP_LOOP_NEXT]] = add nuw i32 [[OMP_LOOP_IV]], 1
+// CHECK-NEXT:    br label [[OMP_LOOP_HEADER]], !llvm.loop [[LOOP4:![0-9]+]]
+// CHECK:       omp_loop.exit:
+// CHECK-NEXT:    br label [[OMP_LOOP_AFTER:%.*]]
+// CHECK:       omp_loop.after:
+// CHECK-NEXT:    store i32 3, i32* [[J]], align 4
+// CHECK-NEXT:    [[TMP11:%.*]] = getelementptr inbounds [[STRUCT_ANON_1]], %struct.anon.1* [[AGG_CAPTURED8]], i32 0, i32 0
+// CHECK-NEXT:    store i32* [[J]], i32** [[TMP11]], align 8
+// CHECK-NEXT:    [[TMP12:%.*]] = getelementptr inbounds [[STRUCT_ANON_2]], %struct.anon.2* [[AGG_CAPTURED9]], i32 0, i32 0
+// CHECK-NEXT:    [[TMP13:%.*]] = load i32, i32* [[J]], align 4
+// CHECK-NEXT:    store i32 [[TMP13]], i32* [[TMP12]], align 4
+// CHECK-NEXT:    call void @__captured_stmt.2(i32* [[DOTCOUNT_ADDR10]], %struct.anon.1* [[AGG_CAPTURED8]])
+// CHECK-NEXT:    [[DOTCOUNT11:%.*]] = load i32, i32* [[DOTCOUNT_ADDR10]], align 4
+// CHECK-NEXT:    br label [[OMP_LOOP_PREHEADER12:%.*]]
+// CHECK:       omp_loop.preheader12:
+// CHECK-NEXT:    br label [[OMP_LOOP_HEADER13:%.*]]
+// CHECK:       omp_loop.header13:
+// CHECK-NEXT:    [[OMP_LOOP_IV19:%.*]] = phi i32 [ 0, [[OMP_LOOP_PREHEADER12]] ], [ [[OMP_LOOP_NEXT21:%.*]], [[OMP_LOOP_INC16:%.*]] ]
+// CHECK-NEXT:    br label [[OMP_LOOP_COND14:%.*]]
+// CHECK:       omp_loop.cond14:
+// CHECK-NEXT:    [[OMP_LOOP_CMP20:%.*]] = icmp ult i32 [[OMP_LOOP_IV19]], [[DOTCOUNT11]]
+// CHECK-NEXT:    br i1 [[OMP_LOOP_CMP20]], label [[OMP_LOOP_BODY15:%.*]], label [[OMP_LOOP_EXIT17:%.*]]
+// CHECK:       omp_loop.body15:
+// CHECK-NEXT:    call void @__captured_stmt.3(i32* [[J]], i32 [[OMP_LOOP_IV19]], %struct.anon.2* [[AGG_CAPTURED9]]), !llvm.access.group [[ACC_GRP8:![0-9]+]]
+// CHECK-NEXT:    [[A22:%.*]] = getelementptr inbounds [[STRUCT_P]], %struct.P* [[PP]], i32 0, i32 0
+// CHECK-NEXT:    [[TMP14:%.*]] = load i32, i32* [[A22]], align 4, !llvm.access.group [[ACC_GRP8]]
+// CHECK-NEXT:    [[TMP15:%.*]] = load i32*, i32** [[C_ADDR]], align 8, !llvm.access.group [[ACC_GRP8]]
+// CHECK-NEXT:    [[TMP16:%.*]] = load i32, i32* [[J]], align 4, !llvm.access.group [[ACC_GRP8]]
+// CHECK-NEXT:    [[IDXPROM23:%.*]] = sext i32 [[TMP16]] to i64
+// CHECK-NEXT:    [[ARRAYIDX24:%.*]] = getelementptr inbounds i32, i32* [[TMP15]], i64 [[IDXPROM23]]
+// CHECK-NEXT:    store i32 [[TMP14]], i32* [[ARRAYIDX24]], align 4, !llvm.access.group [[ACC_GRP8]]
+// CHECK-NEXT:    br label [[OMP_LOOP_INC16]]
+// CHECK:       omp_loop.inc16:
+// CHECK-NEXT:    [[OMP_LOOP_NEXT21]] = add nuw i32 [[OMP_LOOP_IV19]], 1
+// CHECK-NEXT:    br label [[OMP_LOOP_HEADER13]], !llvm.loop [[LOOP9:![0-9]+]]
+// CHECK:       omp_loop.exit17:
+// CHECK-NEXT:    br label [[OMP_LOOP_AFTER18:%.*]]
+// CHECK:       omp_loop.after18:
+// CHECK-NEXT:    ret void
+//
+void simple(float *a, float *b, int *c) {
+  S s, *p;
+  P pp;
+#pragma omp simd simdlen(3)
+  for (int i = 3; i < 32; i += 5) {
+    a[i] = b[i] + s.a + p->a;
+  }
+
+#pragma omp simd
+  for (int j = 3; j < 32; j += 5) {
+    c[j] = pp.a;
+  }
+}
+//.
+// CHECK: attributes #0 = { mustprogress noinline nounwind optnone "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
+// CHECK: attributes #1 = { noinline nounwind optnone "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
+//.
+// CHECK: !0 = !{i32 1, !"wchar_size", i32 4}
+// CHECK: !1 = !{i32 7, !"openmp", i32 45}
+// CHECK: !3 = distinct !{}
+// CHECK: !4 = distinct !{!4, !5, !6, !7}
+// CHECK: !5 = !{!"llvm.loop.parallel_accesses", !3}
+// CHECK: !6 = !{!"llvm.loop.vectorize.enable", i1 true}
+// CHECK: !7 = !{!"llvm.loop.vectorize.width", i32 3}
+// CHECK: !8 = distinct !{}
+// CHECK: !9 = distinct !{!9, !10, !6}
+// CHECK: !10 = !{!"llvm.loop.parallel_accesses", !8}
+//.

llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

public:
   /// \param UnrolledCLI If non-null, receives the CanonicalLoopInfo of the
   ///                    partially unrolled loop. Otherwise, uses loop metadata
   ///                    to defer unrolling to the LoopUnrollPass.
   void unrollLoopPartial(DebugLoc DL, CanonicalLoopInfo *Loop, int32_t Factor,
                          CanonicalLoopInfo **UnrolledCLI);
 
   /// Add metadata to simd-ize a loop.
   ///
-  /// \param DL   Debug location for instructions added by unrolling.
-  /// \param Loop The loop to simd-ize.
-  void applySimd(DebugLoc DL, CanonicalLoopInfo *Loop);
+  /// \param Loop    The loop to simd-ize.
+  /// \param Simdlen The Simdlen length to apply to the simd loop.
+  void applySimd(CanonicalLoopInfo *Loop, ConstantInt *Simdlen);
 
   /// Generator for '#omp flush'
   ///
   /// \param Loc The location where the flush directive was encountered
   void createFlush(const LocationDescription &Loc);
 
   /// Generator for '#omp taskwait'
   ///
   /// \param Loc The location where the taskwait directive was encountered.
   void createTaskwait(const LocationDescription &Loc);
 
   /// Generator for '#omp taskyield'
   ///
   /// \param Loc The location where the taskyield directive was encountered.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

 void OpenMPIRBuilder::unrollLoopHeuristic(DebugLoc, CanonicalLoopInfo *Loop) {
   LLVMContext &Ctx = Builder.getContext();
   addLoopMetadata(
       Loop, {
                 MDNode::get(Ctx, MDString::get(Ctx, "llvm.loop.unroll.enable")),
             });
 }
 
-void OpenMPIRBuilder::applySimd(DebugLoc, CanonicalLoopInfo *CanonicalLoop) {
+void OpenMPIRBuilder::applySimd(CanonicalLoopInfo *CanonicalLoop,
+                                ConstantInt *Simdlen) {
   LLVMContext &Ctx = Builder.getContext();
 
   Function *F = CanonicalLoop->getFunction();
 
   FunctionAnalysisManager FAM;
   FAM.registerPass([]() { return DominatorTreeAnalysis(); });
   FAM.registerPass([]() { return LoopAnalysis(); });
   FAM.registerPass([]() { return PassInstrumentationAnalysis(); });
void OpenMPIRBuilder::applySimd(CanonicalLoopInfo *CanonicalLoop,
   // TODO:  If the loop has existing parallel access metadata, have
   // to combine two lists.
   addLoopMetadata(
       CanonicalLoop,
       {MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.parallel_accesses"),
                          AccessGroup}),
        MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.vectorize.enable"),
                          BoolConst})});
+  if (Simdlen != nullptr)
+    addLoopMetadata(
+        CanonicalLoop,
+        MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.vectorize.width"),
+                          ConstantAsMetadata::get(Simdlen)}));
 }
 
 /// Create the TargetMachine object to query the backend for optimization
 /// preferences.
 ///
 /// Ideally, this would be passed from the front-end to the OpenMPBuilder, but
 /// e.g. Clang does not pass it to its CodeGen layer and creates it only when
 /// needed for the LLVM pass pipline. We use some default options to avoid
 /// having to pass too many settings from the frontend that probably do not
 /// matter.
 ///
 /// Currently, TargetMachine is only used sometimes by the unrollLoopPartial

llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

 }
 
 TEST_F(OpenMPIRBuilderTest, ApplySimd) {
   OpenMPIRBuilder OMPBuilder(*M);
 
   CanonicalLoopInfo *CLI = buildSingleLoopFunction(DL, OMPBuilder, 32);
 
   // Simd-ize the loop.
-  OMPBuilder.applySimd(DL, CLI);
+  OMPBuilder.applySimd(CLI, nullptr);
 
   OMPBuilder.finalize();
   EXPECT_FALSE(verifyModule(*M, &errs()));
 
   PassBuilder PB;
   FunctionAnalysisManager FAM;
   PB.registerFunctionAnalyses(FAM);
   LoopInfo &LI = FAM.getResult<LoopAnalysis>(*F);
 
   const std::vector<Loop *> &TopLvl = LI.getTopLevelLoops();
   EXPECT_EQ(TopLvl.size(), 1u);
 
   Loop *L = TopLvl.front();
   EXPECT_TRUE(findStringMetadataForLoop(L, "llvm.loop.parallel_accesses"));
   EXPECT_TRUE(getBooleanLoopAttribute(L, "llvm.loop.vectorize.enable"));
 
   // Check for llvm.access.group metadata attached to the printf
   // function in the loop body.
   BasicBlock *LoopBody = CLI->getBody();
   EXPECT_TRUE(any_of(*LoopBody, [](Instruction &I) {
     return I.getMetadata("llvm.access.group") != nullptr;
   }));
 }
 
+TEST_F(OpenMPIRBuilderTest, ApplySimdlen) {
+  OpenMPIRBuilder OMPBuilder(*M);
+
+  CanonicalLoopInfo *CLI = buildSingleLoopFunction(DL, OMPBuilder, 32);
+
+  // Simd-ize the loop.
+  OMPBuilder.applySimd(CLI, ConstantInt::get(Type::getInt32Ty(Ctx), 3));
+
+  OMPBuilder.finalize();
+  EXPECT_FALSE(verifyModule(*M, &errs()));
+
+  PassBuilder PB;
+  FunctionAnalysisManager FAM;
+  PB.registerFunctionAnalyses(FAM);
+  LoopInfo &LI = FAM.getResult<LoopAnalysis>(*F);
+
+  const std::vector<Loop *> &TopLvl = LI.getTopLevelLoops();
+  EXPECT_EQ(TopLvl.size(), 1u);
+
+  Loop *L = TopLvl.front();
+  EXPECT_TRUE(findStringMetadataForLoop(L, "llvm.loop.parallel_accesses"));
+  EXPECT_TRUE(getBooleanLoopAttribute(L, "llvm.loop.vectorize.enable"));
+  EXPECT_EQ(getIntLoopAttribute(L, "llvm.loop.vectorize.width"), 3);
+
+  // Check for llvm.access.group metadata attached to the printf
+  // function in the loop body.
+  BasicBlock *LoopBody = CLI->getBody();
+  EXPECT_TRUE(any_of(*LoopBody, [](Instruction &I) {
+    return I.getMetadata("llvm.access.group") != nullptr;
+  }));
+}
+
 TEST_F(OpenMPIRBuilderTest, UnrollLoopFull) {
   OpenMPIRBuilder OMPBuilder(*M);
 
   CanonicalLoopInfo *CLI = buildSingleLoopFunction(DL, OMPBuilder, 32);
 
   // Unroll the loop.
   OMPBuilder.unrollLoopFull(DL, CLI);
 

llvm/utils/UpdateTestChecks/common.py

def get_value_use(self, var, match, var_prefix=None):
     if self.is_local_def_ir_value_match(match):
       return capture_start + self.get_value_name(var, var_prefix) + ']]'
     prefix = self.get_ir_prefix_from_ir_value_match(match)[0]
     return prefix + capture_start + self.get_value_name(var, var_prefix) + ']]'
 
 # Description of the different "unnamed" values we match in the IR, e.g.,
 # (local) ssa values, (debug) metadata, etc.
 ir_nameless_values = [
-    NamelessValue(r'TMP'  , '%' , r'%'           , None            , None                   , r'[\w$.-]+?' , None                 , False) ,
-    NamelessValue(r'ATTR' , '#' , r'#'           , None            , None                   , r'[0-9]+'    , None                 , False) ,
-    NamelessValue(r'ATTR' , '#' , None           , r'attributes #' , r'[0-9]+'              , None         , r'{[^}]*}'           , False) ,
-    NamelessValue(r'GLOB' , '@' , r'@'           , None            , None                   , r'[0-9]+'    , None                 , False) ,
-    NamelessValue(r'GLOB' , '@' , None           , r'@'            , r'[a-zA-Z0-9_$"\\.-]+' , None         , r'.+'                , True)  ,
-    NamelessValue(r'DBG'  , '!' , r'!dbg '       , None            , None                   , r'![0-9]+'   , None                 , False) ,
-    NamelessValue(r'PROF' , '!' , r'!prof '      , None            , None                   , r'![0-9]+'   , None                 , False) ,
-    NamelessValue(r'TBAA' , '!' , r'!tbaa '      , None            , None                   , r'![0-9]+'   , None                 , False) ,
-    NamelessValue(r'RNG'  , '!' , r'!range '     , None            , None                   , r'![0-9]+'   , None                 , False) ,
-    NamelessValue(r'LOOP' , '!' , r'!llvm.loop ' , None            , None                   , r'![0-9]+'   , None                 , False) ,
-    NamelessValue(r'META' , '!' , r'metadata '   , None            , None                   , r'![0-9]+'   , None                 , False) ,
-    NamelessValue(r'META' , '!' , None           , r''             , r'![0-9]+'             , None         , r'(?:distinct |)!.*' , False) ,
+    NamelessValue(r'TMP'     , '%' , r'%'                   , None            , None                   , r'[\w$.-]+?' , None                 , False) ,
+    NamelessValue(r'ATTR'    , '#' , r'#'                   , None            , None                   , r'[0-9]+'    , None                 , False) ,
+    NamelessValue(r'ATTR'    , '#' , None                   , r'attributes #' , r'[0-9]+'              , None         , r'{[^}]*}'           , False) ,
+    NamelessValue(r'GLOB'    , '@' , r'@'                   , None            , None                   , r'[0-9]+'    , None                 , False) ,
+    NamelessValue(r'GLOB'    , '@' , None                   , r'@'            , r'[a-zA-Z0-9_$"\\.-]+' , None         , r'.+'                , True)  ,
+    NamelessValue(r'DBG'     , '!' , r'!dbg '               , None            , None                   , r'![0-9]+'   , None                 , False) ,
+    NamelessValue(r'PROF'    , '!' , r'!prof '              , None            , None                   , r'![0-9]+'   , None                 , False) ,
+    NamelessValue(r'TBAA'    , '!' , r'!tbaa '              , None            , None                   , r'![0-9]+'   , None                 , False) ,
+    NamelessValue(r'RNG'     , '!' , r'!range '             , None            , None                   , r'![0-9]+'   , None                 , False) ,
+    NamelessValue(r'LOOP'    , '!' , r'!llvm.loop '         , None            , None                   , r'![0-9]+'   , None                 , False) ,
+    NamelessValue(r'META'    , '!' , r'metadata '           , None            , None                   , r'![0-9]+'   , None                 , False) ,
+    NamelessValue(r'META'    , '!' , None                   , r''             , r'![0-9]+'             , None         , r'(?:distinct |)!.*' , False) ,
+    NamelessValue(r'ACC_GRP' , '!' , r'!llvm.access.group ' , None            , None                   , r'![0-9]+'   , None                 , False) ,
 ]
 
 asm_nameless_values = [
  NamelessValue(r'MCINST', 'Inst#', None, '<MCInst #', r'\d+', None, r'.+',
                False, is_number=True, replace_number_with_counter=True),
  NamelessValue(r'MCREG',  'Reg:', None, '<MCOperand Reg:', r'\d+', None, r'.+',
                False, is_number=True, replace_number_with_counter=True),
 ]

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

if (failed(bodyGenStatus))
       return failure();
   }
 
   // Collapse loops.
   llvm::IRBuilderBase::InsertPoint afterIP = loopInfos.front()->getAfterIP();
   llvm::CanonicalLoopInfo *loopInfo =
       ompBuilder->collapseLoops(ompLoc.DL, loopInfos, {});
 
-  ompBuilder->applySimd(ompLoc.DL, loopInfo);
+  ompBuilder->applySimd(loopInfo, nullptr);
 
   builder.restoreIP(afterIP);
   return success();
 }
 
 /// Convert an Atomic Ordering attribute to llvm::AtomicOrdering.
 llvm::AtomicOrdering
 convertAtomicOrdering(Optional<omp::ClauseMemoryOrderKind> ao) {