[all-commits] [llvm/llvm-project] 3eaa53: Reapply "[IRBuilder] Virtualize IRBuilder"

Nikita Popov via All-commits all-commits at lists.llvm.org
Mon Feb 17 10:18:04 PST 2020


  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: 3eaa53e80543813a2274391956e91d275950fbf8
      https://github.com/llvm/llvm-project/commit/3eaa53e80543813a2274391956e91d275950fbf8
  Author: Nikita Popov <nikita.ppv at gmail.com>
  Date:   2020-02-17 (Mon, 17 Feb 2020)

  Changed paths:
    M clang/lib/CodeGen/CGBuilder.h
    M llvm/include/llvm/Analysis/TargetFolder.h
    M llvm/include/llvm/IR/ConstantFolder.h
    M llvm/include/llvm/IR/IRBuilder.h
    A llvm/include/llvm/IR/IRBuilderFolder.h
    M llvm/include/llvm/IR/NoFolder.h
    M llvm/lib/Analysis/ConstantFolding.cpp
    M llvm/lib/IR/IRBuilder.cpp
    M llvm/lib/Transforms/Scalar/SROA.cpp
    M polly/include/polly/CodeGen/IRBuilder.h

  Log Message:
  -----------
  Reapply "[IRBuilder] Virtualize IRBuilder"

Relative to the original commit, this fixes some warnings,
and is based on the deletion of the IRBuilder copy constructor
in D74693. The automatic copy constructor would no longer be
safe.

-----

Related llvm-dev thread:
http://lists.llvm.org/pipermail/llvm-dev/2020-February/138951.html

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:

1. It's not possible to share code between use-sites that use
different IRBuilder folders/inserters (short of templating the code
and moving it into headers).
2. Methods currently defined on IRBuilderBase (which is not templated)
do not use the custom inserter, resulting in subtle bugs (e.g.
incorrect InstCombine worklist management). It would be possible to
move those into the templated IRBuilder, but...
3. The vast majority of the IRBuilder implementation has to live
in the header, because it depends on the template arguments.
4. We have many unnecessary dependencies on IRBuilder.h,
because it is not easy to forward-declare. (Significant parts of
the backend depend on it via TargetLowering.h, for example.)

This patch addresses the issue by making the following changes:

* IRBuilderDefaultInserter::InsertHelper becomes virtual.
  IRBuilderBase accepts a reference to it.
* IRBuilderFolder is introduced as a virtual base class. It is
 implemented by ConstantFolder (default), NoFolder and TargetFolder.
  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.
* 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 should be mostly transparent:
The only thing that consumers using a custom inserted may need to do is
inherit from IRBuilderDefaultInserter publicly and mark their InsertHelper
as public.

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




More information about the All-commits mailing list