Mark store and load of block invoke function as invariant.group

[yaxunl]

Summary

block invoke function once is stored in block literal, it will not change.
Therefore mark store and load of block invoke function as invariant.group.

This will facilitate lowering of indirect function calls to direct function
calls, which is important for OpenCL since OpenCL does not require
function pointer support.

[rjmccall]

lib/CodeGen/CGBlocks.cpp

1324

OpenCL blocks are still potentially function-local, right? I don't think you're allowed to put invariant.load on something that's visibly initialized, even if it's visibly initialized to the same thing every time. The problem is that invariant.load could allow the load to be hoisted above the initialization.

If you can solve that problem, you can make this non-OpenCL-specific.

[yaxunl]

lib/CodeGen/CGBlocks.cpp

1324

It seems that invariant.load implies the pointer is invariant in the whole module, disregarding the store to it, which is not suitable for this case.

In this case what I need is that after the first store to the pointer, every load does not change.

It seems invariant.group can be used for this.

[yaxunl]

mark store and load of block invoke function as invariant.group.

[Anastasia]

Btw, blocks w/o captures are already optimized into regular calls?

[rjmccall]

In D53295#1271590, @Anastasia wrote:

Btw, blocks w/o captures are already optimized into regular calls?

That's a very easy optimization for the optimizer to do because the global can be marked constant.

lib/CodeGen/CGBlocks.cpp

980

Please match the capitalization of local variable names used in the surrounding code.

1324

Hmm. I guess invariant.group does what you need because LLVM will probably unify the GEPs to extract the function-pointer field if they appear in the same function, and there are no launderings of that pointer.

CC'ing Piotr to get his opinion about whether this is a correct use of invariant.group. Piotr, we have a struct-typed alloca that we initialize in a probably-dominating position. A pointer to the struct can be passed off to other contexts which we can't necessarily analyze, blocking normal optimization; however, we know that this struct can (mostly) not be legally modified after initialization. We're marking one of the initializing stores as well as the load of the corresponding field when it's used. These two places are completely separate and will perform their own GEPs, but because we don't emit any launders on the struct, LLVM will likely unify the GEPs if they appear in the same function, allowing invariant.group-based optimizations to apply. Does this seem reasonable?

test/CodeGenOpenCL/blocks-indirect-call.cl

11

I don't know what this TODO means.

[Prazek]

lib/CodeGen/CGBlocks.cpp

1324

Yep, seems totally reasonable. Sorry for such a late response, I just have open reviews and saw that.

[rjmccall]

Great, thank you. Yaxun, are you planning to pick this back up? I know it's been a long time.

[yaxunl]

In D53295#1561890, @rjmccall wrote:

Great, thank you. Yaxun, are you planning to pick this back up? I know it's been a long time.

Sorry I caught up with some other work. Currently there has been another change about block which eliminates most indirect block calls, therefore the need for this patch is not so imminent now.

[rjmccall]

Okay. Akira, do you have any interest in looking into this as a general block optimization?

lib/CodeGen/CGBlocks.cpp

if (!IsOpenCL) {
     if (blockInfo.getBlockDecl()->doesNotEscape())
       flags |= BLOCK_IS_NOESCAPE | BLOCK_IS_GLOBAL;
   }
 
   auto projectField =
     [&](unsigned index, CharUnits offset, const Twine &name) -> Address {
       return Builder.CreateStructGEP(blockAddr, index, offset, name);
     };
-  auto storeField =
-    [&](llvm::Value *value, unsigned index, CharUnits offset,
-        const Twine &name) {
-      Builder.CreateStore(value, projectField(index, offset, name));
-    };
+  auto storeField = [&](llvm::Value *value, unsigned index, CharUnits offset,
+                        const Twine &name, bool IsInvariant) {
+    auto *ST = Builder.CreateStore(value, projectField(index, offset, name));
+    if (IsInvariant)
+      ST->setMetadata(CGM.getModule().getMDKindID("invariant.group"),
+                      llvm::MDNode::get(getLLVMContext(), None));
 
   // Initialize the block header.
   {
     // We assume all the header fields are densely packed.
     unsigned index = 0;
     CharUnits offset;
-    auto addHeaderField =
-      [&](llvm::Value *value, CharUnits size, const Twine &name) {
-        storeField(value, index, offset, name);
-        offset += size;
-        index++;
-      };
+    auto addHeaderField = [&](llvm::Value *value, CharUnits size,
+                              const Twine &name, bool IsInvariant = false) {
+      storeField(value, index, offset, name, IsInvariant);
 
     if (!IsOpenCL) {
       addHeaderField(isa, getPointerSize(), "block.isa");
       addHeaderField(llvm::ConstantInt::get(IntTy, flags.getBitMask()),
                      getIntSize(), "block.flags");
       addHeaderField(llvm::ConstantInt::get(IntTy, 0), getIntSize(),
                      "block.reserved");
     } else {
       addHeaderField(
           llvm::ConstantInt::get(IntTy, blockInfo.BlockSize.getQuantity()),
           getIntSize(), "block.size");
       addHeaderField(
           llvm::ConstantInt::get(IntTy, blockInfo.BlockAlign.getQuantity()),
           getIntSize(), "block.align");
     }
-    addHeaderField(blockFn, GenVoidPtrSize, "block.invoke");
+    addHeaderField(blockFn, GenVoidPtrSize, "block.invoke",
+                   /*IsInvariant=*/true);
     if (!IsOpenCL)
       addHeaderField(descriptor, getPointerSize(), "block.descriptor");
     else if (auto *Helper =
                  CGM.getTargetCodeGenInfo().getTargetOpenCLBlockHelper()) {
       for (auto I : Helper->getCustomFieldValues(*this, blockInfo)) {
         addHeaderField(
             I.first,
             CharUnits::fromQuantity(
RValue CodeGenFunction::EmitBlockCallExpr(const CallExpr *E,
 
   QualType FnType = BPT->getPointeeType();
 
   // And the rest of the arguments.
   EmitCallArgs(Args, FnType->getAs<FunctionProtoType>(), E->arguments());
 
   // Load the function.
   llvm::Value *Func = Builder.CreateAlignedLoad(FuncPtr, getPointerAlign());
+  cast<llvm::LoadInst>(Func)->setMetadata(
+      CGM.getModule().getMDKindID("invariant.group"),
+      llvm::MDNode::get(getLLVMContext(), None));
 
   const FunctionType *FuncTy = FnType->castAs<FunctionType>();
   const CGFunctionInfo &FnInfo =
     CGM.getTypes().arrangeBlockFunctionCall(Args, FuncTy);
 
   // Cast the function pointer to the right type.
   llvm::Type *BlockFTy = CGM.getTypes().GetFunctionType(FnInfo);
 
   llvm::Type *BlockFTyPtr = llvm::PointerType::getUnqual(BlockFTy);
   Func = Builder.CreatePointerCast(Func, BlockFTyPtr);
 
   // Prepare the callee.
   CGCallee Callee(CGCalleeInfo(), Func);
 
   // And call the block.
   return EmitCall(FnInfo, Callee, ReturnValue, Args);
 }
 
 Address CodeGenFunction::GetAddrOfBlockDecl(const VarDecl *variable) {
   assert(BlockInfo && "evaluating block ref without block information?");
   const CGBlockInfo::Capture &capture = BlockInfo->getCapture(variable);
 

test/CodeGenOpenCL/blocks-indirect-call.cl

+// RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -o - -O0 -triple amdgcn-amd-amdhsa | FileCheck -check-prefixes=COMMON,NOOPT %s
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -o - -triple amdgcn-amd-amdhsa | FileCheck -check-prefixes=COMMON,OPT %s
+
+// Check invariant.group MD are emitted at -O0 and indirect function calls
+// are eliminated at default optimization level.
+
+// COMMON-LABEL: define {{.*}} @blockInLoopCondition
+// NOOPT: store {{.*}}@__blockInLoopCondition_block_invoke{{.*}} !invariant.group
+// NOOPT: %[[INV:.*]] = getelementptr {{.*}}%block.literal, i32 0, i32 2
+// NOOPT: load {{.*}}%[[INV]]{{.*}}, !invariant.group
+// ToDo: Fix LLVM optimizations to lower indirect function call
+// OPT: %[[INV:.*]] = phi {{.*}}[ @__blockInLoopCondition_block_invoke, %entry ]
+// OPT: call i32 %[[INV]]
+
+// OPT-NOT: load {{.*}}!invariant.group
+void blockInLoopCondition(int* res, int tid, int multiplier) {
+  int (^kernelBlock)(int) = ^(int num) {
+    return num * multiplier;
+  };
+  res[tid] = 39;
+  for(int i=0; i<kernelBlock(13); i++) {
+    res[tid]--;
+  }
+}
+
+// COMMON-LABEL: define {{.*}} @blockInLoopBody
+// NOOPT: store {{.*}}@__blockInLoopBody_block_invoke{{.*}} !invariant.group
+// NOOPT: %[[INV:.*]] = getelementptr {{.*}}%block.literal, i32 0, i32 2
+// NOOPT: load {{.*}}%[[INV]]{{.*}}, !invariant.group
+// OPT: call i32 @__blockInLoopBody_block_invoke
+// OPT-NOT: load {{.*}}!invariant.group
+void blockInLoopBody(int* res, int tid, int multiplier) {
+  res[tid] = 39;
+  for(int i=0; i<1000; i++) {
+    int (^kernelBlock)(int) = ^(int num) {
+      return num * multiplier;
+    };
+    res[tid]=kernelBlock(tid);
+  }
+}
+