[PATCH] D73835: [IRBuilder] Virtualize IRBuilder

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: spatel, lebedev.ri, efriedma.
Herald added a reviewer: bollu.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

This patch moves the IRBuilder from templating over the constant folder and inserter towards making both of these virtual. There are a couple of motivations for this:

- Currently, it's hard/impossible to share code using IRBuilders with different folders/inserters. The original motivation was to make the SimplifyLibCalls utility (which further depends on the BuildLibCall utility) make use of the InstCombine IR builder (which uses a custom inserter). However, there isn't really any way to do that without bleeding the used inserter across the codebase.
- There are some creation methods on BaseBuilder. However, these **do not** make use of the inserter and perform manual insertion instead, making the inserter callback unreliable. This happened because the BaseBuilder just doesn't have access to the inserter...
- Nearly the entirety of IRBuilder is implemented as inline methods in the header. This is a hefty 3k loc file and very little of it looks like code that benefits from being inlined. However, because it depends on the template arguments, it cannot be moved into the source file.

This patch addresses the issue by making the following changes:

- `IRBuilderDefaultInserter::InsertHelper` becomes virtual. The `IRBuilderBase` accepts a reference to it.
- `IRBuilderFolder` is introduced as a virtual base class. It is implemented by `ConstantFolder` (default), `NoFolder` and `TargetFolder`. The `IRBuilderBase` has a reference to this as well.
- All the logic is moved from `IRBuilder` to `IRBuilderBase`. This means that methods can in the future replace their `IRBuilder<> &` uses (or other specific IRBuilder types) with `IRBuilderBase &` and thus be usable with different IRBuilders.
- The `IRBuilder` class is now a thin wrapper around `IRBuilderBase`. Essentially it only stores the folder and inserter and takes care of constructing the base builder.

What this patch doesn't do, but should be simple followups after this change:

- Fixing use of the inserter for creation methods originally defined on `IRBuilderBase` (this is a trivial change, keeping it out of here to keep things NFC).
- Replacing `IRBuilder<>` uses in arguments with `IRBuilderBase`, where useful.
- Moving code from the IRBuilder header to the source file.

>From the user perspective, these changes are mostly transparent: The only thing that consumers may need to do is mark their `InsertHelper` as public instead of protected. (This patch currently only has the LLVM part, one or two adjustments like that will be needed in clang/polly).

What do you think?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73835

Files:
  llvm/include/llvm/Analysis/TargetFolder.h
  llvm/include/llvm/IR/ConstantFolder.h
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/IR/IRBuilderFolder.h
  llvm/include/llvm/IR/NoFolder.h
  llvm/lib/Transforms/Scalar/SROA.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D73835.241890.patch
Type: text/x-patch
Size: 59390 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200201/a24372b8/attachment.bin>