[all-commits] [llvm/llvm-project] ff289f: [OpenMPIRBuilder] Remove ContinuationBB argument f...

[Michael Kruse via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: ff289feeba402a7cf68444e05e4ad2f1e5816767
      https://github.com/llvm/llvm-project/commit/ff289feeba402a7cf68444e05e4ad2f1e5816767
  Author: Michael Kruse <llvm-project at meinersbur.de>
  Date:   2022-04-26 (Tue, 26 Apr 2022)

  Changed paths:
    M clang/lib/CodeGen/CGStmtOpenMP.cpp
    M clang/lib/CodeGen/CodeGenFunction.h
    M clang/test/OpenMP/cancel_codegen.cpp
    M clang/test/OpenMP/critical_codegen.cpp
    M clang/test/OpenMP/critical_codegen_attr.cpp
    M clang/test/OpenMP/masked_codegen.cpp
    M clang/test/OpenMP/master_codegen.cpp
    M clang/test/OpenMP/ordered_codegen.cpp
    M llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
    M llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
    M llvm/lib/Transforms/IPO/OpenMPOpt.cpp
    M llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
    M mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
    M mlir/test/Target/LLVMIR/openmp-llvm.mlir

  Log Message:
  -----------
  [OpenMPIRBuilder] Remove ContinuationBB argument from Body callback.

The callback is expected to create a branch to the ContinuationBB (sometimes called FiniBB in some lambdas) argument when finishing. This creates problems:

 1. The InsertPoint used for CodeGenIP does not need to be the end of a block. If it is not, a naive callback will insert a branch instruction into the middle of the block.

 2. The BasicBlock the CodeGenIP is pointing to may or may not have a terminator. There is an conflict where to branch to if the block already has a terminator.

 3. Some API functions work only with block having a terminator. Some workarounds have been used to insert a temporary terminator that is removed again.

 4. Some callbacks are sensitive to whether the BasicBlock has a terminator or not. This creates a callback ordering problem where different callback may have different behaviour depending on whether a previous callback created a terminator or not. The problem also exists for FinalizeCallbackTy where some callbacks do create branch to another "continue" block, but unlike BodyGenCallbackTy does not receive the target as argument. This is not addressed in this patch.

With this patch, the callback receives an CodeGenIP into a BasicBlock where to insert instructions. If it has to insert control flow, it can split the block at that position as needed but otherwise no separate ContinuationBB is needed. In particular, a callback can be empty without breaking the emitted IR. If the caller needs the control flow to branch to a specific target, it can insert the branch instruction itself and pass an InsertPoint before the terminator to the callback.

Certain frontends such as Clang may expect the current IRBuilder position to be at the end of a basic block. In this case its callbacks must split the block at CodeGenIP before setting the IRBuilder position such that the instructions after CodeGenIP are moved to another basic block and before returning create a new branch instruction to the split block.

Some utility functions such as `splitBB` are supporting correct splitting of BasicBlocks, independent of whether they have a terminator or not, returning/setting the InsertPoint of an IRBuilder to the end of split predecessor block, and optionally omitting creating a branch to the split successor block to be added later.

Reviewed By: kiranchandramohan

Differential Revision: https://reviews.llvm.org/D118409