[OpenMPIRBuilder] forward arguments as pointers to outlined function

[ftynse]

Summary

OpenMPIRBuilder::createParallel outlines the body region of the parallel
construct into a new function that accepts any value previously defined outside
the region as a function argument. This function is called back by OpenMP
runtime function __kmpc_fork_call, which expects trailing arguments to be
pointers. If the region uses a value that is not of a pointer type, e.g. a
struct, the produced code would be invalid. In such cases, make createParallel
emit IR that stores the value on stack and pass the pointer to the outlined
function instead. The outlined function then loads the value back and uses as
normal.

[ftynse]

I ran into this when trying to generate OpenMP code from MLIR (not Fortran). We use structs for our rich pointer abstraction and using that struct in the outlined code led to memory corruptions because it was not passed by pointer. I suppose the intent is the same as https://reviews.llvm.org/D91556, so consider me commandeering that revision.

[ftynse]

Update tests

[Meinersbur]

Looking at CodeGenFunction::GenerateOpenMPCapturedVars, the condition if (!CurField->getType()->isAnyPointerType()) seems to match what is done here, but I wonder whether it is always necessary. Is the additional indirection necessary for pointer-sized data types (e.g. intptr_t, or smaller)?

Aside this optimization, the change looks like something has to be done. Can you update the naming conventions pointed-out by clang-tidy?

[llitchev]

This looks fine. Except the failing tests. Once the tests are fixed it should be good to go.

I have the changes on https://reviews.llvm.org/D91556 made (talked offline with Johannes and made changes to create the struct and capture the values just before the extraction as well - the parameters captured are the ones that the definition of the used vars are parented by a different block than the block of the region. Spending time doing bunch of testing. The approach in this PR works as well.

[fghanim]

Thank you for the patch :)

To be honest, I am not exactly sure what is the problem you are trying to address here, is it about passing shared variables as proper pointers? or is it about creating locals for said arguments

Based on the patch description; I think it is the first; and AFAIK, the current implementation will always pass shared variables correctly as pointers. is that not the case? I mean I have never came across a case where it didn't
If yes, do you have a c/c++ example I can try? I'd be very interested in seeing for which cases this doesn't work.

However, implementation and checks makes me think it is the second, Which I always assumed was going to be a part of how a specific frontend handles privatization; but I am very much not against doing so here
Just will want to make sure to do it in a way that works for the OMP-IR, and I don't know if there is or what is the plan to handle private/shared variables is on their end.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

701

Why not use InnerAllocaIP instead? it's in the entry block of the parallel region.

707

NIT: IPGuard maybe better?

712

For the alloca, should use OuterAllocaIP or InnerAllocaIP based on which function the alloca belongs to.

allocations in InsertBB will end right before the fork call, which suggests you probably should use outerAllocaIP. but why do you need two allocations for same variable. or is this meant to be the allocations where we store the arguments in the outlined function? if that's the case, then you should use InnerAllocaIP

713

Nit: if you could specify a name for the alloca, a name is easier to track than a number in IR :)

[jdoerfert]

Nit: Clang tidy warning wrt variable names should be addressed. Some other nits inlined.

My only concern is described in the longer inline comment.

---

In D92189#2424216, @fghanim wrote:

To be honest, I am not exactly sure what is the problem you are trying to address here, is it about passing shared variables as proper pointers? or is it about creating locals for said arguments

The problem was discussed in the Flang + OpenMP call lately and got attention by at least 4 people since then, as far as I can tell independently. I'm rather confused TBH, but it's a "good thing", as far as bug fixes can be good ;)

Long story short, kmpc_fork_call expects variadic arguments to be in integer registers, anything else passed is lost and replaced with garbage. Clang deals with this already but since the problem is not unique to Clang we should do it here. If anyone wants to create a parallel region they should not need to know the implicit calling convention rules of the OpenMP runtime. For an example how clang "optimizes" things, see https://godbolt.org/z/cja5TE, especially the float/i32 bitcasts.

Based on the patch description; I think it is the first; and AFAIK, the current implementation will always pass shared variables correctly as pointers. is that not the case? I mean I have never came across a case where it didn't
If yes, do you have a c/c++ example I can try? I'd be very interested in seeing for which cases this doesn't work.

It's not about shared variables but basically the equivalent of "firstprivate" (=pass by value), though it occurs in non-frontend contexts as well, e.g. when merging/expanding parallel regions.

However, implementation and checks makes me think it is the second, Which I always assumed was going to be a part of how a specific frontend handles privatization; but I am very much not against doing so here

It belongs here and I asked them to put it here instead of Flang, especially since @ggeorgakoudis exposed the same problem in OpenMP opt. Now instead of fixing up pointers 3 times and exposing a very fragile API to non-OpenMP users, we will ensure the calling convention is not leaking out of the IRBuilder.

Just will want to make sure to do it in a way that works for the OMP-IR, and I don't know if there is or what is the plan to handle private/shared variables is on their end.

Privatization (in all its glory) has still to happen in the frontend. This is only partially related.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

742

I was expecting the above logic to be placed here, after the PrivCB callback as I assumed we would privatize in the sequential part. Clang does not and we do not either (for now), which is unfortunate. It does duplicate the privatization and makes this patch more complicated than I anticipated. I though we can simply replace ReplacementValue by Reloaded if ReplacementValue wasn't a pointer and therefor put in an alloca. The fact that we need to reload V and then "replace twice" seems really unfortunate.

What would happen if you do not have Reloaded at all but instead make it V = createlodoad(alloca)? After all, Reloaded is equivalent to V in all aspects, right? Would this work? Would we still need the code below? I feel like there must be a minimal solution as we only want to replace the value once on the other side of the call edge.

760

Nit: Assertion message please.

780

Nit: -"we"

[jdoerfert]

In D92189#2420932, @Meinersbur wrote:

Looking at CodeGenFunction::GenerateOpenMPCapturedVars, the condition if (!CurField->getType()->isAnyPointerType()) seems to match what is done here, but I wonder whether it is always necessary. Is the additional indirection necessary for pointer-sized data types (e.g. intptr_t, or smaller)?

Aside this optimization, the change looks like something has to be done. Can you update the naming conventions pointed-out by clang-tidy?

Let's keep it simple. TBH, I am not convinced Clang's "optimization" is not hurting us in the long run. I mean, we'd need to understand this: bitcast float to i32 -> zext ---fork_call---> trunc -> bitcast i32 to float while the pointer indirection is common also for shared values so we will be (actually are downstream) able to handle those.

[jdoerfert]

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

742

-"duplicate privatization" +"duplicate replace all uses with"

[ftynse]

Address reviews.

[ftynse]

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

701

I started with that, but it did not work out. InnerAllocaIP is used by PrivCB to construct IR that may be using the value defined at ReloadIP. If I literally replace the use of ReloadIP with InnerAllocaIP, the instructions with have the wrong order. I also considered inserting this at the top of the entry block, but this messes up the order of arguments in the outlined function. Suggestions on how to structure this better are welcome.

712

This belongs to the outer function, but OuterAllocaIP does not seem valid at this point anymore. Using it leads to segfaults, I can investigate later, after we decide whether this code should remain here or move below as Johannes suggested.

742

I am not sure I fully follow what you suggest, so I will elaborate on the two versions I had considered.

1. Move the code that loads back the value (currently lines 709-725) after this line. This will not remove the need for two separate "replace all uses with": there uses of the original V in the body that need to be replaced with ReplacementValue, and there are uses of V that could have been introduced by PrivCB for the purpose of defining ReplacementValue which should be replaced with Reloaded instead. This doesn't look like it would address your concern about having double replacement.

2. I can keep the code that loads back the value in its current place and pass either V or *Reloaded to PrivCB. This will make sure any instructions created in PrivCB use the Reloaded and don't need to be update via "replace all uses with" pattern. However, this exposes the pointer mechanism to the caller of CreateParallel. The Value & that PrivCB receives is not necessarily a value that exists in the user code, it can be the alloca we constructed in builder. So the implementation of PrivCB needs to be aware of it and can no longer rely on, e.g., keeping a list of values/instructions that need privatization or directly rely on types since the type would change. I decided that I did not want to change the contract that PrivCB has with CreateParallel because I was not aware of all its use cases (and have a preference for more "isolated" APIs) . However, if you think it is acceptable for the builder in order to reduce the complexity/overhead of the code, I can adapt.

There is a flavor of (2) that changes PrivCB to take both V and Replacement so that the implementation of PrivCB can easily detect when the pointer mechanism is active.

[ftynse]

Fix clang tests. The order of arguments is switched in the internal outlined function.

[ftynse]

clang/test/OpenMP/parallel_codegen.cpp

143

The order of arguments changes here because we create a use of the promoted-to-pointer argument before any other uses in the body and the outliner finds it first. This should be fine because it's just an internal outlined function that the builder created and the calls to it are emitted accordingly and in the same builder. I can add a comment that explains this if desired.

If we go with Michael's suggestion not to turn into pointers the integer values whose size is equal to or smaller than pointer size, this change will not be necessary. I seem to remember seeing some documentation that says that trailing arguments to fork_call should be _pointers_, but I can't find it anymore.

[jdoerfert]

While only partially related, can you leave a FIXME saying that more than 15 arguments need to be packed in a structure?

clang/test/OpenMP/parallel_codegen.cpp

143

I seem to remember seeing some documentation that says that trailing arguments to fork_call should be _pointers_, but I can't find it anymore.

Technically, anything that is passed in the same register as a void * is fine. The documentation on this is thin at best. As I mentioned in my response to @Meinersbur, I think turning everything into pointers is a reasonable way to handle this. I gave some rational there (https://reviews.llvm.org/D92189#2424690)

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

742

Appreciate the detailed explanations!

There is a flavor of (2) that changes PrivCB to take both V and Replacement so that the implementation of PrivCB can easily detect when the pointer mechanism is active.

This looks like a sweet-spot. We can avoid adding more replacement logic but the PrivCB is aware of what is going on. If you are OK with this solution as well I'd prefer it.

[ftynse]

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

742

Great, will do this.

[llitchev]

In D92189#2425639, @jdoerfert wrote:

While only partially related, can you leave a FIXME saying that more than 15 arguments need to be packed in a structure?

Yes, the number of varargs that is guarantee to work is 15, if more are needed, there is a need to wrap the parameters in a struct.
As we discussed (bring it here too) the issue here is how parameters are passed following the calling convention. The varargs assumes that parameters are passed in GPR (INTEGER classifications based on the calling conventions). The varargs approach works fine for the INTEGER classified args. It breaks for the SSE(UP) and vector clarified parameters. If we pass the pointers to the SSE params (floats for example) everything will work - pointers are INTEGER classification. For Windows things are a bit different of course, based on the specific calling convention used there.

[ftynse]

Simplify the code by adapting the PrivatizationCallback signature.

[jdoerfert]

LGTM, one comment and two nits. Feel free to commit if you agree with the suggestions or come back with concerns. Thanks for working on this and taking the time to fix it like this!

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

124

It is not a pointer to original, is it? It is an equivalent but potentially not the same IR value.

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

738

Nit: Rename Reloaded to Inner to make it consistent with the callback (description). With Value* Inner = V; you can avoid the select above.

745

Nit: This could go back up again I guess.

[fghanim]

Thanks; just two more mediocre things if possible. if not. you are good to go. :)

@jdoerfert

In D92189#2424677, @jdoerfert wrote:

In D92189#2424216, @fghanim wrote:

To be honest, I am not exactly sure what is the problem you are trying to address here, is it about passing shared variables as proper pointers? or is it about creating locals for said arguments

The problem was discussed in the Flang + OpenMP call lately and got attention by at least 4 people since then, as far as I can tell independently. I'm rather confused TBH, but it's a "good thing", as far as bug fixes can be good ;)

I don't disagree. As I said, "I am very much not against doing so here"

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

701

OK; The only instructions generated at InnerAllocaIP up to this point are the things generated in bodyCB, are these the users that you are talking about?

In any case, and since you already reset InnerAllocaIP to ReloadIP, I prefer we do that before privatization (i.e. reset InnerAllocaIP, to ZeroAddrUse->getNextNode() directly), and use that instead.

712

That's probably because the OuterAllocaIP iterator got invalidated between the beginning and here.

If you want; get the insertion BasicBlock early on, and then use that to reupdate OuterAllocaIP and use that to place whatever you want. right now those alloca.s are not in an entry block.

[ftynse]

Address all remaining review comments.

[ftynse]

Fix a runaway MLIR use of createParallel

[ftynse]

Windows failure is unrelated, broken by an earlier commit to MLIR PDL.

clang/lib/CodeGen/CGStmtOpenMP.cpp

auto FiniCB = [this](InsertPointTy IP) {
       OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
     };
 
     // Privatization callback that performs appropriate action for
     // shared/private/firstprivate/lastprivate/copyin/... variables.
     //
     // TODO: This defaults to shared right now.
     auto PrivCB = [](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
-                     llvm::Value &Val, llvm::Value *&ReplVal) {
+                     llvm::Value &, llvm::Value &Val, llvm::Value *&ReplVal) {
       // The next line is appropriate only for variables (Val) with the
       // data-sharing attribute "shared".
       ReplVal = &Val;
 
       return CodeGenIP;
     };
 
     const CapturedStmt *CS = S.getCapturedStmt(OMPD_parallel);

clang/test/OpenMP/parallel_codegen.cpp

 
 // CHECK:           define internal {{.*}}void [[OMP_OUTLINED21]](i32* noalias %{{.+}}, i32* noalias %{{.+}}, i{{[0-9]+}}{{.*}} [[VLA_SIZE:%.+]], i32* {{.+}} [[VLA_ADDR:%[^)]+]])
 // CHECK:           load i32, i32* @
 
 // ALL-DEBUG-DAG: declare !callback ![[cbid:[0-9]+]] void @__kmpc_fork_call(%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...)
 // CHECK-DEBUG-DAG:       define internal void [[OMP_OUTLINED]](i32* noalias %.global_tid., i32* noalias %.bound_tid., i64 [[VLA_SIZE:%.+]], i32* {{.+}} [[VLA_ADDR:%[^)]+]])
 // CHECK-DEBUG-DAG:       call void [[OMP_OUTLINED_DEBUG]]
 
+// Note that OpenMPIRBuilder puts the trailing arguments in a different order:
+// arguments that are wrapped into additional pointers precede the other
+// arguments. This is expected and not problematic because both the call and the
+// function are generated from the same place, and the function is internal.
 // ALL:       define linkonce_odr {{[a-z\_\b]*[ ]?i32}} [[TMAIN]](i8** %argc)
 // ALL:       store i8** %argc, i8*** [[ARGC_ADDR:%.+]],
 // CHECK:       call {{.*}}void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* [[DEF_LOC_2]], i32 2, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, i8***, i{{64|32}})* [[OMP_OUTLINED:@.+]] to void (i32*, i32*, ...)*), i8*** [[ARGC_ADDR]], i{{64|32}} %{{.+}})
-// IRBUILDER:   call {{.*}}void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* [[DEF_LOC_2]], i32 2, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, i8***, i{{64|32}})* [[OMP_OUTLINED:@.+]] to void (i32*, i32*, ...)*), i8*** [[ARGC_ADDR]], i{{64|32}} %{{.+}})
+// IRBUILDER:   call {{.*}}void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* [[DEF_LOC_2]], i32 2, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, i{{64|32}}*, i8***)* [[OMP_OUTLINED:@.+]] to void (i32*, i32*, ...)*), i{{64|32}}* %{{.+}}, i8*** [[ARGC_ADDR]])
 // ALL:  ret i32 0
 // ALL-NEXT:  }
 // ALL-DEBUG:       define linkonce_odr i32 [[TMAIN]](i8** %argc)
 
 // CHECK-DEBUG:       store i8** %argc, i8*** [[ARGC_ADDR:%.+]],
 // CHECK-DEBUG:       call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @{{.*}}, i32 2, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, i8***, i64)* [[OMP_OUTLINED:@.+]] to void (i32*, i32*, ...)*), i8*** [[ARGC_ADDR]], i64 %{{.+}})
-// IRBUILDER-DEBUG:   call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @{{.*}}, i32 2, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, i8***, i64)* [[OMP_OUTLINED:@.+]] to void (i32*, i32*, ...)*), i8*** [[ARGC_ADDR]], i64 %{{.+}})
+// IRBUILDER-DEBUG:   call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @{{.*}}, i32 2, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, i64*, i8***)* [[OMP_OUTLINED:@.+]] to void (i32*, i32*, ...)*), i64* %{{.+}}, i8*** [[ARGC_ADDR]])
 // ALL-DEBUG:  ret i32 0
 // ALL-DEBUG-NEXT:  }
 
 // CHECK:       define internal {{.*}}void [[OMP_OUTLINED]](i32* noalias %.global_tid., i32* noalias %.bound_tid., i8*** nonnull align {{[0-9]+}} dereferenceable({{4|8}}) %argc, i{{64|32}}{{.*}} %{{.+}})
-// IRBUILDER:   define internal {{.*}}void [[OMP_OUTLINED]](i32* noalias %{{.*}}, i32* noalias %{{.*}}, i8*** [[ARGC_REF:%.*]], i{{64|32}}{{.*}} %{{.+}})
+// IRBUILDER:   define internal {{.*}}void [[OMP_OUTLINED]](i32* noalias %{{.*}}, i32* noalias %{{.*}}, i{{64|32}}*{{.*}} %{{.+}}, i8*** [[ARGC_REF:%.*]])
 // CHECK:       store i8*** %argc, i8**** [[ARGC_PTR_ADDR:%.+]],
 // CHECK:       [[ARGC_REF:%.+]] = load i8***, i8**** [[ARGC_PTR_ADDR]]
 // ALL:         [[ARGC:%.+]] = load i8**, i8*** [[ARGC_REF]]
 // CHECK-NEXT:  invoke {{.*}}void [[FOO1:@.+foo.+]](i8** [[ARGC]])
 // IRBUILDER-NEXT: call {{.*}}void [[FOO1:@.+foo.+]](i8** [[ARGC]])
 // CHECK:       ret void
 // CHECK:       call {{.*}}void @{{.+terminate.*|abort}}(
 // CHECK-NEXT:  unreachable
 // CHECK-NEXT:  }
 // CHECK-DEBUG:       define internal void [[OMP_OUTLINED_DEBUG:@.+]](i32* noalias %.global_tid., i32* noalias %.bound_tid., i8*** nonnull align {{[0-9]+}} dereferenceable({{4|8}}) %argc, i64 %{{.+}})
-// IRBUILDER-DEBUG:   define internal void [[OMP_OUTLINED_DEBUG:@.+]](i32* noalias %{{.*}}, i32* noalias %{{.*}}, i8*** [[ARGC_REF:%.*]], i64 %{{.+}})
+// IRBUILDER-DEBUG:   define internal void [[OMP_OUTLINED_DEBUG:@.+]](i32* noalias %{{.*}}, i32* noalias %{{.*}}, i64* %{{.+}}, i8*** [[ARGC_REF:%.*]])
 // CHECK-DEBUG:       store i8*** %argc, i8**** [[ARGC_PTR_ADDR:%.+]],
 // CHECK-DEBUG:       [[ARGC_REF:%.+]] = load i8***, i8**** [[ARGC_PTR_ADDR]]
 // ALL-DEBUG:         [[ARGC:%.+]] = load i8**, i8*** [[ARGC_REF]]
 // CHECK-DEBUG-NEXT:  invoke void [[FOO1:@.+foo.+]](i8** [[ARGC]])
 // IRBUILDER-DEBUG-NEXT:  call void [[FOO1:@.+foo.+]](i8** [[ARGC]])
 // CHECK-DEBUG:       ret void
 // CHECK-DEBUG:       call void @{{.+terminate.*|abort}}(
 // CHECK-DEBUG-NEXT:  unreachable

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

public:
 
   /// Callback type for variable privatization (think copy & default
   /// constructor).
   ///
   /// \param AllocaIP is the insertion point at which new alloca instructions
   ///                 should be placed.
   /// \param CodeGenIP is the insertion point at which the privatization code
   ///                  should be placed.
-  /// \param Val The value beeing copied/created.
+  /// \param Original The value being copied/created, should not be used in the
+  ///                 generated IR.
+  /// \param Inner The equivalent of \p Original that should be used in the
+  ///              generated IR; this is equal to \p Original if the value is
+  ///              a pointer and can thus be passed directly, otherwise it is
+  ///              an equivalent but different value.
   /// \param ReplVal The replacement value, thus a copy or new created version
-  ///                of \p Val.
+  ///                of \p Inner.
   ///
   /// \returns The new insertion point where code generation continues and
-  ///          \p ReplVal the replacement of \p Val.
+  ///          \p ReplVal the replacement value.
   using PrivatizeCallbackTy = function_ref<InsertPointTy(
-      InsertPointTy AllocaIP, InsertPointTy CodeGenIP, Value &Val,
-      Value *&ReplVal)>;
+      InsertPointTy AllocaIP, InsertPointTy CodeGenIP, Value &Original,
+      Value &Inner, Value *&ReplVal)>;
 
   /// Description of a LLVM-IR insertion point (IP) and a debug/source location
   /// (filename, line, column, ...).
   struct LocationDescription {
     template <typename T, typename U>
     LocationDescription(const IRBuilder<T, U> &IRB)
         : IP(IRB.saveIP()), DL(IRB.getCurrentDebugLocation()) {}
     LocationDescription(const InsertPointTy &IP) : IP(IP) {}

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Value *Args[] = {
         ConstantInt::get(Int32, unsigned(ProcBind), /*isSigned=*/true)};
     Builder.CreateCall(
         getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_push_proc_bind), Args);
   }
 
   BasicBlock *InsertBB = Builder.GetInsertBlock();
   Function *OuterFn = InsertBB->getParent();
 
+  // Save the outer alloca block because the insertion iterator may get
+  // invalidated and we still need this later.
+  BasicBlock *OuterAllocaBlock = OuterAllocaIP.getBlock();
+
   // Vector to remember instructions we used only during the modeling but which
   // we want to delete at the end.
   SmallVector<Instruction *, 4> ToBeDeleted;
 
   // Change the location to the outer alloca insertion point to create and
   // initialize the allocas we pass into the parallel region.
   Builder.restoreIP(OuterAllocaIP);
   AllocaInst *TIDAddr = Builder.CreateAlloca(Int32, nullptr, "tid.addr");
IRBuilder<>::InsertPoint OpenMPIRBuilder::createParallel(
   InsertPointTy InnerAllocaIP = Builder.saveIP();
 
   AllocaInst *PrivTIDAddr =
       Builder.CreateAlloca(Int32, nullptr, "tid.addr.local");
   Instruction *PrivTID = Builder.CreateLoad(PrivTIDAddr, "tid");
 
   // Add some fake uses for OpenMP provided arguments.
   ToBeDeleted.push_back(Builder.CreateLoad(TIDAddr, "tid.addr.use"));
-  ToBeDeleted.push_back(Builder.CreateLoad(ZeroAddr, "zero.addr.use"));
+  Instruction *ZeroAddrUse = Builder.CreateLoad(ZeroAddr, "zero.addr.use");
+  ToBeDeleted.push_back(ZeroAddrUse);
 
   // ThenBB
   //   |
   //   V
   // PRegionEntryBB         <- Privatization allocas are placed here.
   //   |
   //   V
   // PRegionBodyBB          <- BodeGen is invoked here.
IRBuilder<>::InsertPoint OpenMPIRBuilder::createParallel(
 
   FunctionCallee TIDRTLFn =
       getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_global_thread_num);
 
   auto PrivHelper = [&](Value &V) {
     if (&V == TIDAddr || &V == ZeroAddr)
       return;
 
-    SmallVector<Use *, 8> Uses;
+    SetVector<Use *> Uses;
     for (Use &U : V.uses())
       if (auto *UserI = dyn_cast<Instruction>(U.getUser()))
         if (ParallelRegionBlockSet.count(UserI->getParent()))
-          Uses.push_back(&U);
+          Uses.insert(&U);
+
+    // __kmpc_fork_call expects extra arguments as pointers. If the input
+    // already has a pointer type, everything is fine. Otherwise, store the
+    // value onto stack and load it back inside the to-be-outlined region. This
+    // will ensure only the pointer will be passed to the function.
+    // FIXME: if there are more than 15 trailing arguments, they must be
+    // additionally packed in a struct.
+    Value *Inner = &V;
+    if (!V.getType()->isPointerTy()) {
+      IRBuilder<>::InsertPointGuard Guard(Builder);
+      LLVM_DEBUG(llvm::dbgs() << "Forwarding input as pointer: " << V << "\n");
+
+      Builder.restoreIP(OuterAllocaIP);
+      Value *Ptr =
+          Builder.CreateAlloca(V.getType(), nullptr, V.getName() + ".reloaded");
+
+      // Store to stack at end of the block that currently branches to the entry
+      // block of the to-be-outlined region.
+      Builder.SetInsertPoint(InsertBB,
+                             InsertBB->getTerminator()->getIterator());
+      Builder.CreateStore(&V, Ptr);
+
+      // Load back next to allocations in the to-be-outlined region.
+      Builder.restoreIP(InnerAllocaIP);
+      Inner = Builder.CreateLoad(Ptr);
+    }
 
     Value *ReplacementValue = nullptr;
     CallInst *CI = dyn_cast<CallInst>(&V);
     if (CI && CI->getCalledFunction() == TIDRTLFn.getCallee()) {
       ReplacementValue = PrivTID;
     } else {
       Builder.restoreIP(
-          PrivCB(InnerAllocaIP, Builder.saveIP(), V, ReplacementValue));
+          PrivCB(InnerAllocaIP, Builder.saveIP(), V, *Inner, ReplacementValue));
       assert(ReplacementValue &&
              "Expected copy/create callback to set replacement value!");
       if (ReplacementValue == &V)
         return;
     }
 
     for (Use *UPtr : Uses)
       UPtr->set(ReplacementValue);
   };
 
+  // Reset the inner alloca insertion as it will be used for loading the values
+  // wrapped into pointers before passing them into the to-be-outlined region.
+  // Configure it to insert immediately after the fake use of zero address so
+  // that they are available in the generated body and so that the
+  // OpenMP-related values (thread ID and zero address pointers) remain leading
+  // in the argument list.
+  InnerAllocaIP = IRBuilder<>::InsertPoint(
+      ZeroAddrUse->getParent(), ZeroAddrUse->getNextNode()->getIterator());
+
+  // Reset the outer alloca insertion point to the entry of the relevant block
+  // in case it was invalidated.
+  OuterAllocaIP = IRBuilder<>::InsertPoint(
+      OuterAllocaBlock, OuterAllocaBlock->getFirstInsertionPt());
+
   for (Value *Input : Inputs) {
     LLVM_DEBUG(dbgs() << "Captured input: " << *Input << "\n");
     PrivHelper(*Input);
   }
   LLVM_DEBUG({
     for (Value *Output : Outputs)
       LLVM_DEBUG(dbgs() << "Captured output: " << *Output << "\n");
   });
   assert(Outputs.empty() &&
          "OpenMP outlining should not produce live-out values!");
 
   LLVM_DEBUG(dbgs() << "After  privatization: " << *OuterFn << "\n");
   LLVM_DEBUG({
     for (auto *BB : Blocks)
       dbgs() << " PBR: " << BB->getName() << "\n";
   });
 
   // Register the outlined info.
   addOutlineInfo(std::move(OI));
 
   InsertPointTy AfterIP(UI->getParent(), UI->getParent()->end());
   UI->eraseFromParent();
 
   return AfterIP;
 }
 
 void OpenMPIRBuilder::emitFlush(const LocationDescription &Loc) {

llvm/lib/Transforms/IPO/OpenMPOpt.cpp

auto BodyGenCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
       BasicBlock *CGEndBB =
           SplitBlock(CGStartBB, &*CodeGenIP.getPoint(), DT, LI);
       assert(StartBB != nullptr && "StartBB should not be null");
       CGStartBB->getTerminator()->setSuccessor(0, StartBB);
       assert(EndBB != nullptr && "EndBB should not be null");
       EndBB->getTerminator()->setSuccessor(0, CGEndBB);
     };
 
-    auto PrivCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
-                      Value &VPtr, Value *&ReplacementValue) -> InsertPointTy {
-      ReplacementValue = &VPtr;
+    auto PrivCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP, Value &,
+                      Value &Inner, Value *&ReplacementValue) -> InsertPointTy {
+      ReplacementValue = &Inner;
       return CodeGenIP;
     };
 
     auto FiniCB = [&](InsertPointTy CodeGenIP) {};
 
     // Helper to merge the __kmpc_fork_call calls in MergableCIs. They are all
     // contained in BB and only separated by instructions that can be
     // redundantly executed in parallel. The block BB is split before the first

llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

protected:
 
   LLVMContext Ctx;
   std::unique_ptr<Module> M;
   Function *F;
   BasicBlock *BB;
   DebugLoc DL;
 };
 
+// Returns the value stored in the given allocation. Returns null if the given
+// value is not a result of an allocation, if no value is stored or if there is
+// more than one store.
+static Value *findStoredValue(Value *AllocaValue) {
+  Instruction *Alloca = dyn_cast<AllocaInst>(AllocaValue);
+  if (!Alloca)
+    return nullptr;
+  StoreInst *Store = nullptr;
+  for (Use &U : Alloca->uses()) {
+    if (auto *CandidateStore = dyn_cast<StoreInst>(U.getUser())) {
+      EXPECT_EQ(Store, nullptr);
+      Store = CandidateStore;
+    }
+  }
+  if (!Store)
+    return nullptr;
+  return Store->getValueOperand();
+};
+
 TEST_F(OpenMPIRBuilderTest, CreateBarrier) {
   OpenMPIRBuilder OMPBuilder(*M);
   OMPBuilder.initialize();
 
   IRBuilder<> Builder(BB);
 
   OMPBuilder.createBarrier({IRBuilder<>::InsertPoint()}, OMPD_for);
   EXPECT_TRUE(M->global_empty());
SplitBlockAndInsertIfThenElse(Cmp, CodeGenIP.getBlock()->getTerminator(),
                                   &ThenTerm, &ElseTerm);
 
     Builder.SetInsertPoint(ThenTerm);
     Builder.CreateBr(&ContinuationIP);
     ThenTerm->eraseFromParent();
   };
 
   auto PrivCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
-                    Value &VPtr, Value *&ReplacementValue) -> InsertPointTy {
+                    Value &Orig, Value &Inner,
+                    Value *&ReplacementValue) -> InsertPointTy {
     ++NumPrivatizedVars;
 
-    if (!isa<AllocaInst>(VPtr)) {
-      EXPECT_EQ(&VPtr, F->arg_begin());
-      ReplacementValue = &VPtr;
+    if (!isa<AllocaInst>(Orig)) {
+      EXPECT_EQ(&Orig, F->arg_begin());
+      ReplacementValue = &Inner;
       return CodeGenIP;
     }
 
+    // Since the original value is an allocation, it has a pointer type and
+    // therefore no additional wrapping should happen.
+    EXPECT_EQ(&Orig, &Inner);
+
     // Trivial copy (=firstprivate).
     Builder.restoreIP(AllocaIP);
-    Type *VTy = VPtr.getType()->getPointerElementType();
-    Value *V = Builder.CreateLoad(VTy, &VPtr, VPtr.getName() + ".reload");
-    ReplacementValue = Builder.CreateAlloca(VTy, 0, VPtr.getName() + ".copy");
+    Type *VTy = Inner.getType()->getPointerElementType();
+    Value *V = Builder.CreateLoad(VTy, &Inner, Orig.getName() + ".reload");
+    ReplacementValue = Builder.CreateAlloca(VTy, 0, Orig.getName() + ".copy");
     Builder.restoreIP(CodeGenIP);
     Builder.CreateStore(V, ReplacementValue);
     return CodeGenIP;
   };
 
   auto FiniCB = [&](InsertPointTy CodeGenIP) { ++NumFinalizationPoints; };
 
   IRBuilder<>::InsertPoint AllocaIP(&F->getEntryBlock(),
TEST_F(OpenMPIRBuilderTest, ParallelSimple) {
   ASSERT_NE(ForkCI, nullptr);
 
   EXPECT_EQ(ForkCI->getCalledFunction()->getName(), "__kmpc_fork_call");
   EXPECT_EQ(ForkCI->getNumArgOperands(), 4U);
   EXPECT_TRUE(isa<GlobalVariable>(ForkCI->getArgOperand(0)));
   EXPECT_EQ(ForkCI->getArgOperand(1),
             ConstantInt::get(Type::getInt32Ty(Ctx), 1U));
   EXPECT_EQ(ForkCI->getArgOperand(2), Usr);
-  EXPECT_EQ(ForkCI->getArgOperand(3), F->arg_begin());
+  EXPECT_EQ(findStoredValue(ForkCI->getArgOperand(3)), F->arg_begin());
 }
 
 TEST_F(OpenMPIRBuilderTest, ParallelNested) {
   using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
   OpenMPIRBuilder OMPBuilder(*M);
   OMPBuilder.initialize();
   F->setName("func");
   IRBuilder<> Builder(BB);
 
   OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP(), DL});
 
   unsigned NumInnerBodiesGenerated = 0;
   unsigned NumOuterBodiesGenerated = 0;
   unsigned NumFinalizationPoints = 0;
 
   auto InnerBodyGenCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
                             BasicBlock &ContinuationIP) {
     ++NumInnerBodiesGenerated;
   };
 
   auto PrivCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
-                    Value &VPtr, Value *&ReplacementValue) -> InsertPointTy {
+                    Value &Orig, Value &Inner,
+                    Value *&ReplacementValue) -> InsertPointTy {
     // Trivial copy (=firstprivate).
     Builder.restoreIP(AllocaIP);
-    Type *VTy = VPtr.getType()->getPointerElementType();
-    Value *V = Builder.CreateLoad(VTy, &VPtr, VPtr.getName() + ".reload");
-    ReplacementValue = Builder.CreateAlloca(VTy, 0, VPtr.getName() + ".copy");
+    Type *VTy = Inner.getType()->getPointerElementType();
+    Value *V = Builder.CreateLoad(VTy, &Inner, Orig.getName() + ".reload");
+    ReplacementValue = Builder.CreateAlloca(VTy, 0, Orig.getName() + ".copy");
     Builder.restoreIP(CodeGenIP);
     Builder.CreateStore(V, ReplacementValue);
     return CodeGenIP;
   };
 
   auto FiniCB = [&](InsertPointTy CodeGenIP) { ++NumFinalizationPoints; };
 
   auto OuterBodyGenCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
TEST_F(OpenMPIRBuilderTest, ParallelNested2Inner) {
   unsigned NumFinalizationPoints = 0;
 
   auto InnerBodyGenCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
                             BasicBlock &ContinuationIP) {
     ++NumInnerBodiesGenerated;
   };
 
   auto PrivCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
-                    Value &VPtr, Value *&ReplacementValue) -> InsertPointTy {
+                    Value &Orig, Value &Inner,
+                    Value *&ReplacementValue) -> InsertPointTy {
     // Trivial copy (=firstprivate).
     Builder.restoreIP(AllocaIP);
-    Type *VTy = VPtr.getType()->getPointerElementType();
-    Value *V = Builder.CreateLoad(VTy, &VPtr, VPtr.getName() + ".reload");
-    ReplacementValue = Builder.CreateAlloca(VTy, 0, VPtr.getName() + ".copy");
+    Type *VTy = Inner.getType()->getPointerElementType();
+    Value *V = Builder.CreateLoad(VTy, &Inner, Orig.getName() + ".reload");
+    ReplacementValue = Builder.CreateAlloca(VTy, 0, Orig.getName() + ".copy");
     Builder.restoreIP(CodeGenIP);
     Builder.CreateStore(V, ReplacementValue);
     return CodeGenIP;
   };
 
   auto FiniCB = [&](InsertPointTy CodeGenIP) { ++NumFinalizationPoints; };
 
   auto OuterBodyGenCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
SplitBlockAndInsertIfThenElse(Cmp, CodeGenIP.getBlock()->getTerminator(),
                                   &ThenTerm, &ElseTerm);
 
     Builder.SetInsertPoint(ThenTerm);
     Builder.CreateBr(&ContinuationIP);
     ThenTerm->eraseFromParent();
   };
 
   auto PrivCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
-                    Value &VPtr, Value *&ReplacementValue) -> InsertPointTy {
+                    Value &Orig, Value &Inner,
+                    Value *&ReplacementValue) -> InsertPointTy {
     ++NumPrivatizedVars;
 
-    if (!isa<AllocaInst>(VPtr)) {
-      EXPECT_EQ(&VPtr, F->arg_begin());
-      ReplacementValue = &VPtr;
+    if (!isa<AllocaInst>(Orig)) {
+      EXPECT_EQ(&Orig, F->arg_begin());
+      ReplacementValue = &Inner;
       return CodeGenIP;
     }
 
+    // Since the original value is an allocation, it has a pointer type and
+    // therefore no additional wrapping should happen.
+    EXPECT_EQ(&Orig, &Inner);
+
     // Trivial copy (=firstprivate).
     Builder.restoreIP(AllocaIP);
-    Type *VTy = VPtr.getType()->getPointerElementType();
-    Value *V = Builder.CreateLoad(VTy, &VPtr, VPtr.getName() + ".reload");
-    ReplacementValue = Builder.CreateAlloca(VTy, 0, VPtr.getName() + ".copy");
+    Type *VTy = Inner.getType()->getPointerElementType();
+    Value *V = Builder.CreateLoad(VTy, &Inner, Orig.getName() + ".reload");
+    ReplacementValue = Builder.CreateAlloca(VTy, 0, Orig.getName() + ".copy");
     Builder.restoreIP(CodeGenIP);
     Builder.CreateStore(V, ReplacementValue);
     return CodeGenIP;
   };
 
   auto FiniCB = [&](InsertPointTy CodeGenIP) {
     ++NumFinalizationPoints;
     // No destructors.
for (User *Usr : OutlinedFn->users()) {
     }
   }
 
   EXPECT_EQ(ForkCI->getCalledFunction()->getName(), "__kmpc_fork_call");
   EXPECT_EQ(ForkCI->getNumArgOperands(), 4U);
   EXPECT_TRUE(isa<GlobalVariable>(ForkCI->getArgOperand(0)));
   EXPECT_EQ(ForkCI->getArgOperand(1),
             ConstantInt::get(Type::getInt32Ty(Ctx), 1));
-  EXPECT_EQ(ForkCI->getArgOperand(3), F->arg_begin());
+  Value *StoredForkArg = findStoredValue(ForkCI->getArgOperand(3));
+  EXPECT_EQ(StoredForkArg, F->arg_begin());
 
   EXPECT_EQ(DirectCI->getCalledFunction(), OutlinedFn);
   EXPECT_EQ(DirectCI->getNumArgOperands(), 3U);
   EXPECT_TRUE(isa<AllocaInst>(DirectCI->getArgOperand(0)));
   EXPECT_TRUE(isa<AllocaInst>(DirectCI->getArgOperand(1)));
-  EXPECT_EQ(DirectCI->getArgOperand(2), F->arg_begin());
+  Value *StoredDirectArg = findStoredValue(DirectCI->getArgOperand(2));
+  EXPECT_EQ(StoredDirectArg, F->arg_begin());
 }
 
 TEST_F(OpenMPIRBuilderTest, ParallelCancelBarrier) {
   using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
   OpenMPIRBuilder OMPBuilder(*M);
   OMPBuilder.initialize();
   F->setName("func");
   IRBuilder<> Builder(BB);
Builder.restoreIP(OMPBuilder.createBarrier(Builder.saveIP(), OMPD_parallel,
                                                false, false));
     ASSERT_EQ(CBFn->getNumUses(), 2U);
     ASSERT_EQ(BFn->getNumUses(), 1U);
     ASSERT_TRUE(CBFn->user_back() != CheckedBarrier);
     ASSERT_TRUE(isa<CallInst>(CBFn->user_back()));
     ASSERT_EQ(CBFn->user_back()->getNumUses(), 0U);
   };
 
-  auto PrivCB = [&](InsertPointTy, InsertPointTy, Value &V,
+  auto PrivCB = [&](InsertPointTy, InsertPointTy, Value &V, Value &,
                     Value *&) -> InsertPointTy {
     ++NumPrivatizedVars;
     llvm_unreachable("No privatization callback call expected!");
   };
 
   FunctionType *FakeDestructorTy =
       FunctionType::get(Type::getVoidTy(Ctx), {Type::getInt32Ty(Ctx)},
                         /*isVarArg=*/false);
if (!isa<ReturnInst>(ExitBB->front())) {
       ASSERT_TRUE(isa<BranchInst>(ExitBB->front()));
       ASSERT_EQ(cast<BranchInst>(ExitBB->front()).getNumSuccessors(), 1U);
       ASSERT_TRUE(isa<ReturnInst>(
           cast<BranchInst>(ExitBB->front()).getSuccessor(0)->front()));
     }
   }
 }
 
+TEST_F(OpenMPIRBuilderTest, ParallelForwardAsPointers) {
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+  F->setName("func");
+  IRBuilder<> Builder(BB);
+  OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP(), DL});
+  using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
+
+  Type *I32Ty = Type::getInt32Ty(M->getContext());
+  Type *I32PtrTy = Type::getInt32PtrTy(M->getContext());
+  Type *StructTy = StructType::get(I32Ty, I32PtrTy);
+  Type *StructPtrTy = StructTy->getPointerTo();
+  Type *VoidTy = Type::getVoidTy(M->getContext());
+  FunctionCallee RetI32Func = M->getOrInsertFunction("ret_i32", I32Ty);
+  FunctionCallee TakeI32Func =
+      M->getOrInsertFunction("take_i32", VoidTy, I32Ty);
+  FunctionCallee RetI32PtrFunc = M->getOrInsertFunction("ret_i32ptr", I32PtrTy);
+  FunctionCallee TakeI32PtrFunc =
+      M->getOrInsertFunction("take_i32ptr", VoidTy, I32PtrTy);
+  FunctionCallee RetStructFunc = M->getOrInsertFunction("ret_struct", StructTy);
+  FunctionCallee TakeStructFunc =
+      M->getOrInsertFunction("take_struct", VoidTy, StructTy);
+  FunctionCallee RetStructPtrFunc =
+      M->getOrInsertFunction("ret_structptr", StructPtrTy);
+  FunctionCallee TakeStructPtrFunc =
+      M->getOrInsertFunction("take_structPtr", VoidTy, StructPtrTy);
+  Value *I32Val = Builder.CreateCall(RetI32Func);
+  Value *I32PtrVal = Builder.CreateCall(RetI32PtrFunc);
+  Value *StructVal = Builder.CreateCall(RetStructFunc);
+  Value *StructPtrVal = Builder.CreateCall(RetStructPtrFunc);
+
+  Instruction *Internal;
+  auto BodyGenCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
+                       BasicBlock &ContinuationBB) {
+    IRBuilder<>::InsertPointGuard Guard(Builder);
+    Builder.restoreIP(CodeGenIP);
+    Internal = Builder.CreateCall(TakeI32Func, I32Val);
+    Builder.CreateCall(TakeI32PtrFunc, I32PtrVal);
+    Builder.CreateCall(TakeStructFunc, StructVal);
+    Builder.CreateCall(TakeStructPtrFunc, StructPtrVal);
+  };
+  auto PrivCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP, Value &,
+                    Value &Inner, Value *&ReplacementValue) {
+    ReplacementValue = &Inner;
+    return CodeGenIP;
+  };
+  auto FiniCB = [](InsertPointTy) {};
+
+  IRBuilder<>::InsertPoint AllocaIP(&F->getEntryBlock(),
+                                    F->getEntryBlock().getFirstInsertionPt());
+  IRBuilder<>::InsertPoint AfterIP =
+      OMPBuilder.createParallel(Loc, AllocaIP, BodyGenCB, PrivCB, FiniCB,
+                                nullptr, nullptr, OMP_PROC_BIND_default, false);
+  Builder.restoreIP(AfterIP);
+  Builder.CreateRetVoid();
+
+  OMPBuilder.finalize();
+
+  EXPECT_FALSE(verifyModule(*M, &errs()));
+  Function *OutlinedFn = Internal->getFunction();
+
+  Type *Arg2Type = OutlinedFn->getArg(2)->getType();
+  EXPECT_TRUE(Arg2Type->isPointerTy());
+  EXPECT_EQ(Arg2Type->getPointerElementType(), I32Ty);
+
+  // Arguments that need to be passed through pointers and reloaded will get
+  // used earlier in the functions and therefore will appear first in the
+  // argument list after outlining.
+  Type *Arg3Type = OutlinedFn->getArg(3)->getType();
+  EXPECT_TRUE(Arg3Type->isPointerTy());
+  EXPECT_EQ(Arg3Type->getPointerElementType(), StructTy);
+
+  Type *Arg4Type = OutlinedFn->getArg(4)->getType();
+  EXPECT_EQ(Arg4Type, I32PtrTy);
+
+  Type *Arg5Type = OutlinedFn->getArg(5)->getType();
+  EXPECT_EQ(Arg5Type, StructPtrTy);
+}
+
 TEST_F(OpenMPIRBuilderTest, CanonicalLoopSimple) {
   using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
   OpenMPIRBuilder OMPBuilder(*M);
   OMPBuilder.initialize();
   IRBuilder<> Builder(BB);
   OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP(), DL});
   Value *TripCount = F->getArg(0);
 

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

auto bodyGenCB = [&](InsertPointTy allocaIP, InsertPointTy codeGenIP,
     // connect the PHI nodes to the results of preceding blocks.
     connectPHINodes(region, valueMapping, blockMapping);
   };
 
   // TODO: Perform appropriate actions according to the data-sharing
   // attribute (shared, private, firstprivate, ...) of variables.
   // Currently defaults to shared.
   auto privCB = [&](InsertPointTy allocaIP, InsertPointTy codeGenIP,
-                    llvm::Value &vPtr,
+                    llvm::Value &, llvm::Value &vPtr,
                     llvm::Value *&replacementValue) -> InsertPointTy {
     replacementValue = &vPtr;
 
     return codeGenIP;
   };
 
   // TODO: Perform finalization actions for variables. This has to be
   // called for variables which have destructors/finalizers.