[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL363620: Rewrite ConstStructBuilder with a mechanism that can 
cope with splitting and… (authored by rsmith, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63371?vs=205182=205184#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63371/new/

https://reviews.llvm.org/D63371

Files:
  cfe/trunk/lib/CodeGen/CGExprConstant.cpp
  cfe/trunk/test/CodeGenCXX/designated-init.cpp

Index: cfe/trunk/test/CodeGenCXX/designated-init.cpp
===
--- cfe/trunk/test/CodeGenCXX/designated-init.cpp
+++ cfe/trunk/test/CodeGenCXX/designated-init.cpp
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -std=c++98 -emit-llvm -o - %s -triple x86_64-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm -o - %s -triple x86_64-linux-gnu | FileCheck %s
+
+struct A { int x, y[3]; };
+struct B { A a; };
+
+// CHECK: @b = global %{{[^ ]*}} { %{{[^ ]*}} { i32 1, [3 x i32] [i32 2, i32 5, i32 4] } }
+B b = {(A){1, 2, 3, 4}, .a.y[1] = 5};
+
+union U {
+  int n;
+  float f;
+};
+struct C {
+  int x;
+  U u[3];
+};
+struct D {
+  C c;
+};
+
+// CHECK: @d1 = {{.*}} { i32 1, [3 x %[[U:.*]]] [%[[U]] { i32 2 }, %[[U]] { i32 5 }, %[[U]] { i32 4 }] }
+D d1 = {(C){1, {{.n=2}, {.f=3}, {.n=4}}}, .c.u[1].n = 5};
+
+// CHECK: @d2 = {{.*}} { i32 1, { %[[U]], float, %[[U]] } { %[[U]] { i32 2 }, float 5.{{0*}}e+00, %[[U]] { i32 4 } } }
+D d2 = {(C){1, 2, 3, 4}, .c.u[1].f = 5};
+
+struct Bitfield {
+  int a : 3;
+  int b : 4;
+  int c : 5;
+};
+struct WithBitfield {
+  int n;
+  Bitfield b;
+};
+// CHECK: @bitfield = {{.*}} { i32 1, { i8, i8, [2 x i8] } { i8 42, i8 2, [2 x i8] undef } }
+WithBitfield bitfield = {1, (Bitfield){2, 3, 4}, .b.b = 5};
+
+struct String {
+  const char buffer[12];
+};
+struct WithString {
+  String str;
+};
+// CHECK: @string = {{.*}} [12 x i8] c"Hello World\00" } }
+WithString string = {(String){"hello world"}, .str.buffer[0] = 'H', .str.buffer[6] = 'W'};
+
+struct LargeArray {
+  int arr[4096];
+};
+struct WithLargeArray {
+  LargeArray arr;
+};
+// CHECK: @large = global { { <{ [11 x i32], [4085 x i32] }> } } { { <{ [11 x i32], [4085 x i32] }> } { <{ [11 x i32], [4085 x i32] }> <{ [11 x i32] [i32 1, i32 2, i32 3, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 10], [4085 x i32] zeroinitializer }> } }
+WithLargeArray large = {(LargeArray){1, 2, 3}, .arr.arr[10] = 10};
+
+union OverwritePaddingWithBitfield {
+  struct Padding { unsigned : 8; char c; } padding;
+  char bitfield : 3;
+};
+struct WithOverwritePaddingWithBitfield {
+  OverwritePaddingWithBitfield a;
+};
+// CHECK: @overwrite_padding = global { { i8, i8 } } { { i8, i8 } { i8 3, i8 1 } }
+WithOverwritePaddingWithBitfield overwrite_padding = {(OverwritePaddingWithBitfield){1}, .a.bitfield = 3};
Index: cfe/trunk/lib/CodeGen/CGExprConstant.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprConstant.cpp
+++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp
@@ -22,6 +22,8 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/Builtins.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Function.h"
@@ -30,345 +32,636 @@
 using namespace CodeGen;
 
 //===--===//
-//ConstStructBuilder
+//ConstantAggregateBuilder
 //===--===//
 
 namespace {
 class ConstExprEmitter;
-class ConstStructBuilder {
-  CodeGenModule 
-  ConstantEmitter 
-
-  bool Packed;
-  CharUnits NextFieldOffsetInChars;
-  CharUnits LLVMStructAlignment;
-  SmallVector Elements;
-public:
-  static llvm::Constant *BuildStruct(ConstantEmitter ,
- ConstExprEmitter *ExprEmitter,
- llvm::Constant *Base,
- InitListExpr *Updater,
- QualType ValTy);
-  static llvm::Constant *BuildStruct(ConstantEmitter ,
- InitListExpr *ILE, QualType StructTy);
-  static llvm::Constant *BuildStruct(ConstantEmitter ,
- const APValue , QualType ValTy);
-
-private:
-  ConstStructBuilder(ConstantEmitter )
-: CGM(emitter.CGM), Emitter(emitter), Packed(false),
-NextFieldOffsetInChars(CharUnits::Zero()),
-LLVMStructAlignment(CharUnits::One()) { }
-
-  void AppendField(const FieldDecl *Field, uint64_t FieldOffset,
-   llvm::Constant *InitExpr);
-
-  void AppendBytes(CharUnits 

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith marked an inline comment as done.
rsmith added inline comments.



Comment at: lib/CodeGen/CGExprConstant.cpp:162
+  return replace(V, BeginOff, EndOff, Vals.begin(), Vals.end());
+}
+

rjmccall wrote:
> Can these go to STLExtras or somewhere similar?
Done. The use of offsets here is a bit special-case, so I've moved an iterator 
version to STLExtras and just left an offsets -> iterators adaptor here.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63371/new/

https://reviews.llvm.org/D63371



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith updated this revision to Diff 205182.
rsmith marked 2 inline comments as done.
rsmith added a comment.

- Address additional review comments from rjmccall.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63371/new/

https://reviews.llvm.org/D63371

Files:
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGenCXX/designated-init.cpp

Index: test/CodeGenCXX/designated-init.cpp
===
--- /dev/null
+++ test/CodeGenCXX/designated-init.cpp
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -std=c++98 -emit-llvm -o - %s -triple x86_64-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm -o - %s -triple x86_64-linux-gnu | FileCheck %s
+
+struct A { int x, y[3]; };
+struct B { A a; };
+
+// CHECK: @b = global %{{[^ ]*}} { %{{[^ ]*}} { i32 1, [3 x i32] [i32 2, i32 5, i32 4] } }
+B b = {(A){1, 2, 3, 4}, .a.y[1] = 5};
+
+union U {
+  int n;
+  float f;
+};
+struct C {
+  int x;
+  U u[3];
+};
+struct D {
+  C c;
+};
+
+// CHECK: @d1 = {{.*}} { i32 1, [3 x %[[U:.*]]] [%[[U]] { i32 2 }, %[[U]] { i32 5 }, %[[U]] { i32 4 }] }
+D d1 = {(C){1, {{.n=2}, {.f=3}, {.n=4}}}, .c.u[1].n = 5};
+
+// CHECK: @d2 = {{.*}} { i32 1, { %[[U]], float, %[[U]] } { %[[U]] { i32 2 }, float 5.{{0*}}e+00, %[[U]] { i32 4 } } }
+D d2 = {(C){1, 2, 3, 4}, .c.u[1].f = 5};
+
+struct Bitfield {
+  int a : 3;
+  int b : 4;
+  int c : 5;
+};
+struct WithBitfield {
+  int n;
+  Bitfield b;
+};
+// CHECK: @bitfield = {{.*}} { i32 1, { i8, i8, [2 x i8] } { i8 42, i8 2, [2 x i8] undef } }
+WithBitfield bitfield = {1, (Bitfield){2, 3, 4}, .b.b = 5};
+
+struct String {
+  const char buffer[12];
+};
+struct WithString {
+  String str;
+};
+// CHECK: @string = {{.*}} [12 x i8] c"Hello World\00" } }
+WithString string = {(String){"hello world"}, .str.buffer[0] = 'H', .str.buffer[6] = 'W'};
+
+struct LargeArray {
+  int arr[4096];
+};
+struct WithLargeArray {
+  LargeArray arr;
+};
+// CHECK: @large = global { { <{ [11 x i32], [4085 x i32] }> } } { { <{ [11 x i32], [4085 x i32] }> } { <{ [11 x i32], [4085 x i32] }> <{ [11 x i32] [i32 1, i32 2, i32 3, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 10], [4085 x i32] zeroinitializer }> } }
+WithLargeArray large = {(LargeArray){1, 2, 3}, .arr.arr[10] = 10};
+
+union OverwritePaddingWithBitfield {
+  struct Padding { unsigned : 8; char c; } padding;
+  char bitfield : 3;
+};
+struct WithOverwritePaddingWithBitfield {
+  OverwritePaddingWithBitfield a;
+};
+// CHECK: @overwrite_padding = global { { i8, i8 } } { { i8, i8 } { i8 3, i8 1 } }
+WithOverwritePaddingWithBitfield overwrite_padding = {(OverwritePaddingWithBitfield){1}, .a.bitfield = 3};
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -22,6 +22,8 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/Builtins.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Function.h"
@@ -30,148 +32,562 @@
 using namespace CodeGen;
 
 //===--===//
-//ConstStructBuilder
+//ConstantAggregateBuilder
 //===--===//
 
 namespace {
 class ConstExprEmitter;
+
+struct ConstantAggregateBuilderUtils {
+  CodeGenModule 
+
+  ConstantAggregateBuilderUtils(CodeGenModule ) : CGM(CGM) {}
+
+  CharUnits getAlignment(const llvm::Constant *C) const {
+return CharUnits::fromQuantity(
+CGM.getDataLayout().getABITypeAlignment(C->getType()));
+  }
+
+  CharUnits getSize(llvm::Type *Ty) const {
+return CharUnits::fromQuantity(CGM.getDataLayout().getTypeAllocSize(Ty));
+  }
+
+  CharUnits getSize(const llvm::Constant *C) const {
+return getSize(C->getType());
+  }
+
+  llvm::Constant *getPadding(CharUnits PadSize) const {
+llvm::Type *Ty = CGM.Int8Ty;
+if (PadSize > CharUnits::One())
+  Ty = llvm::ArrayType::get(Ty, PadSize.getQuantity());
+return llvm::UndefValue::get(Ty);
+  }
+
+  llvm::Constant *getZeroes(CharUnits ZeroSize) const {
+llvm::Type *Ty = llvm::ArrayType::get(CGM.Int8Ty, ZeroSize.getQuantity());
+return llvm::ConstantAggregateZero::get(Ty);
+  }
+};
+
+/// Incremental builder for an llvm::Constant* holding a struct or array
+/// constant.
+class ConstantAggregateBuilder : private ConstantAggregateBuilderUtils {
+  /// The elements of the constant. These two arrays must have the same size;
+  /// Offsets[i] describes the offset of Elems[i] within the constant. The
+  /// elements are kept in increasing offset order, and we ensure that there
+  /// is no overlap: Offsets[i+1] >= Offsets[i] + getSize(Elemes[i]).
+  ///
+  /// This may contain explicit padding elements (in order to 

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Minor requests, then LGTM.




Comment at: lib/CodeGen/CGExprConstant.cpp:73
+/// Incremental builder for an llvm::Constant* holding a structure constant.
+class ConstantBuilder : private ConstantBuilderUtils {
+  llvm::SmallVector Elems;

rsmith wrote:
> rjmccall wrote:
> > This seems like a very generic name for this type.
> It is intended to be a very generic type. (I was trying to arrange it so that 
> it could possibly be moved to LLVM eventually. I heard that globalopt would 
> benefit from being able to do this kind of constant splitting / reforming.) 
> Is `ConstantAggregateBuilder` sufficiently more precise?
Yeah, that makes sense, since it aligns with the LLVM type name.  I guess the 
big blocker there is the lack of a `CharUnits` equivalent in LLVM.



Comment at: lib/CodeGen/CGExprConstant.cpp:190
+bool ConstantBuilder::addBits(llvm::APInt Bits, uint64_t OffsetInBits,
+  bool AllowOverwrite) {
+  const ASTContext  = CGM.getContext();

rsmith wrote:
> rjmccall wrote:
> > `AllowOversized` (which you used in the interface) seems like a better name.
> `AllowOversized` is used to mean "the size of the constant may be larger than 
> the size of the type", and is a parameter to `build` / `buildFrom`.
> `AllowOverwrite` is used to mean "adding this constant may overwrite 
> something you've already been given", and is a parameter to `add` / `addBits`.
> 
> I can make these names more different from each other if that would help?
Oh, oops.  No, sorry, no need to do anything here.



Comment at: lib/CodeGen/CGExprConstant.cpp:258
+  return false;
+assert(CI->getBitWidth() == CharWidth && "splitAt failed");
+assert((!(CI->getValue() & UpdateMask) || AllowOverwrite) &&

rsmith wrote:
> rjmccall wrote:
> > Oh, because we're splitting before and after a single-`CharUnits` range?  
> > That seems worthy of a somewhat clearer explanation in the code.
> > 
> > I guess we could have a non-`ConstantInt` single-byte value.  Unlikely but 
> > not impossible. :)
> Yes. It's not too hard to craft a testcase where we'd get an explicit `undef` 
> here; added handling for that and for all-zeroes constants, which are both 
> correctly handled by overwriting the whole byte.
Ah, yeah, good catch; I was thinking it was okay to be suboptimal if we saw  a 
`<1 x i8>` or a 1-byte pointer or whatever, but undef and zero are definitely 
worth covering here.



Comment at: lib/CodeGen/CGExprConstant.cpp:375
+
+  // FIXME: We could theoretically split a ConstantInt if the need ever arose.
+

rsmith wrote:
> rjmccall wrote:
> > Does this not come up all the time with bit-fields?  I guess we emit them 
> > in single-`char` chunks, so it wouldn't.  Probably worth a comment.
> Done.
> 
> We could hit this case for cases such as:
> 
> ```
> union U { int a; int b : 3; };
> struct S { U u; };
> S s = {(union U){1234}, .u.b = 5};
> ```
> 
> (which `CodeGen` currently rejects with "cannot compile this static 
> initializer yet" in C), and splitting the `ConstantInt` would allow us to 
> emit that initializer as a constant, but I'm not sure it's worthwhile unless 
> it lets us simplify or improve bitfield emission in general. (The above isn't 
> a case that C requires us to treat as a constant initializer, so rejecting it 
> is not a conformance issue.)
> 
> Maybe instead of splitting bitfields into 1-byte chunks like we currently do, 
> we should try to combine them into a single `iN`, like 
> `CGRecordLayoutBuilder` does. But splitting to `i8` maintains the status quo, 
> which is what I was aiming for in this patch.
I'm fine with single-byte emission for constant bit-fields; ugliness here 
shouldn't really have significant consequences.  The comment is good enough.



Comment at: lib/CodeGen/CGExprConstant.cpp:162
+  return replace(V, BeginOff, EndOff, Vals.begin(), Vals.end());
+}
+

Can these go to STLExtras or somewhere similar?



Comment at: lib/CodeGen/CGExprConstant.cpp:221
+// bits have unspecified values.
+llvm::APInt BitsThisByte = Bits;
+if (BitsThisByte.getBitWidth() < CharWidth)

Should this be `BitsThisChar` for consistency?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63371/new/

https://reviews.llvm.org/D63371



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith marked an inline comment as done.
rsmith added inline comments.



Comment at: lib/CodeGen/CGExprConstant.cpp:967
 // Constant folding is currently missing support for a few features supported
 // here: CK_ToUnion, CK_ReinterpretMemberPointer, and DesignatedInitUpdateExpr.
 class ConstExprEmitter :

efriedma wrote:
> Like the comment here says, we're awfully close to being able to just 
> completely kill off ConstExprEmitter etc. Do we really want to continue to 
> invest effort into this code?
The new code is also the mechanism by which we emit `APValue` constants, which 
is the replacement for `ConstExprEmitter`.

Also I don't think we have a fix for the performance issues we saw from 
performing frontend evaluation of large `InitListExpr`s yet, which is another 
blocker for removing `ConstExprEmitter` in favor of `APValue` emission (though 
I don't expect that to be a problem forever; `APValue` is long overdue an 
overhaul).


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63371/new/

https://reviews.llvm.org/D63371



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith updated this revision to Diff 205162.
rsmith added a comment.

- Fix somewhat-incorrect comment on Size member.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63371/new/

https://reviews.llvm.org/D63371

Files:
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGenCXX/designated-init.cpp

Index: test/CodeGenCXX/designated-init.cpp
===
--- /dev/null
+++ test/CodeGenCXX/designated-init.cpp
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -std=c++98 -emit-llvm -o - %s -triple x86_64-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm -o - %s -triple x86_64-linux-gnu | FileCheck %s
+
+struct A { int x, y[3]; };
+struct B { A a; };
+
+// CHECK: @b = global %{{[^ ]*}} { %{{[^ ]*}} { i32 1, [3 x i32] [i32 2, i32 5, i32 4] } }
+B b = {(A){1, 2, 3, 4}, .a.y[1] = 5};
+
+union U {
+  int n;
+  float f;
+};
+struct C {
+  int x;
+  U u[3];
+};
+struct D {
+  C c;
+};
+
+// CHECK: @d1 = {{.*}} { i32 1, [3 x %[[U:.*]]] [%[[U]] { i32 2 }, %[[U]] { i32 5 }, %[[U]] { i32 4 }] }
+D d1 = {(C){1, {{.n=2}, {.f=3}, {.n=4}}}, .c.u[1].n = 5};
+
+// CHECK: @d2 = {{.*}} { i32 1, { %[[U]], float, %[[U]] } { %[[U]] { i32 2 }, float 5.{{0*}}e+00, %[[U]] { i32 4 } } }
+D d2 = {(C){1, 2, 3, 4}, .c.u[1].f = 5};
+
+struct Bitfield {
+  int a : 3;
+  int b : 4;
+  int c : 5;
+};
+struct WithBitfield {
+  int n;
+  Bitfield b;
+};
+// CHECK: @bitfield = {{.*}} { i32 1, { i8, i8, [2 x i8] } { i8 42, i8 2, [2 x i8] undef } }
+WithBitfield bitfield = {1, (Bitfield){2, 3, 4}, .b.b = 5};
+
+struct String {
+  const char buffer[12];
+};
+struct WithString {
+  String str;
+};
+// CHECK: @string = {{.*}} [12 x i8] c"Hello World\00" } }
+WithString string = {(String){"hello world"}, .str.buffer[0] = 'H', .str.buffer[6] = 'W'};
+
+struct LargeArray {
+  int arr[4096];
+};
+struct WithLargeArray {
+  LargeArray arr;
+};
+// CHECK: @large = global { { <{ [11 x i32], [4085 x i32] }> } } { { <{ [11 x i32], [4085 x i32] }> } { <{ [11 x i32], [4085 x i32] }> <{ [11 x i32] [i32 1, i32 2, i32 3, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 10], [4085 x i32] zeroinitializer }> } }
+WithLargeArray large = {(LargeArray){1, 2, 3}, .arr.arr[10] = 10};
+
+union OverwritePaddingWithBitfield {
+  struct Padding { unsigned : 8; char c; } padding;
+  char bitfield : 3;
+};
+struct WithOverwritePaddingWithBitfield {
+  OverwritePaddingWithBitfield a;
+};
+// CHECK: @overwrite_padding = global { { i8, i8 } } { { i8, i8 } { i8 3, i8 1 } }
+WithOverwritePaddingWithBitfield overwrite_padding = {(OverwritePaddingWithBitfield){1}, .a.bitfield = 3};
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -22,6 +22,8 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/Builtins.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Function.h"
@@ -30,148 +32,585 @@
 using namespace CodeGen;
 
 //===--===//
-//ConstStructBuilder
+//ConstantAggregateBuilder
 //===--===//
 
 namespace {
 class ConstExprEmitter;
+
+struct ConstantAggregateBuilderUtils {
+  CodeGenModule 
+
+  ConstantAggregateBuilderUtils(CodeGenModule ) : CGM(CGM) {}
+
+  CharUnits getAlignment(const llvm::Constant *C) const {
+return CharUnits::fromQuantity(
+CGM.getDataLayout().getABITypeAlignment(C->getType()));
+  }
+
+  CharUnits getSize(llvm::Type *Ty) const {
+return CharUnits::fromQuantity(CGM.getDataLayout().getTypeAllocSize(Ty));
+  }
+
+  CharUnits getSize(const llvm::Constant *C) const {
+return getSize(C->getType());
+  }
+
+  llvm::Constant *getPadding(CharUnits PadSize) const {
+llvm::Type *Ty = CGM.Int8Ty;
+if (PadSize > CharUnits::One())
+  Ty = llvm::ArrayType::get(Ty, PadSize.getQuantity());
+return llvm::UndefValue::get(Ty);
+  }
+
+  llvm::Constant *getZeroes(CharUnits ZeroSize) const {
+llvm::Type *Ty = llvm::ArrayType::get(CGM.Int8Ty, ZeroSize.getQuantity());
+return llvm::ConstantAggregateZero::get(Ty);
+  }
+};
+
+/// Incremental builder for an llvm::Constant* holding a struct or array
+/// constant.
+class ConstantAggregateBuilder : private ConstantAggregateBuilderUtils {
+  /// The elements of the constant. These two arrays must have the same size;
+  /// Offsets[i] describes the offset of Elems[i] within the constant. The
+  /// elements are kept in increasing offset order, and we ensure that there
+  /// is no overlap: Offsets[i+1] >= Offsets[i] + getSize(Elemes[i]).
+  ///
+  /// This may contain explicit padding elements (in order to create a
+  /// natural layout), but need not. 

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: lib/CodeGen/CGExprConstant.cpp:967
 // Constant folding is currently missing support for a few features supported
 // here: CK_ToUnion, CK_ReinterpretMemberPointer, and DesignatedInitUpdateExpr.
 class ConstExprEmitter :

Like the comment here says, we're awfully close to being able to just 
completely kill off ConstExprEmitter etc. Do we really want to continue to 
invest effort into this code?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63371/new/

https://reviews.llvm.org/D63371



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: lib/CodeGen/CGExprConstant.cpp:73
+/// Incremental builder for an llvm::Constant* holding a structure constant.
+class ConstantBuilder : private ConstantBuilderUtils {
+  llvm::SmallVector Elems;

rjmccall wrote:
> This seems like a very generic name for this type.
It is intended to be a very generic type. (I was trying to arrange it so that 
it could possibly be moved to LLVM eventually. I heard that globalopt would 
benefit from being able to do this kind of constant splitting / reforming.) Is 
`ConstantAggregateBuilder` sufficiently more precise?



Comment at: lib/CodeGen/CGExprConstant.cpp:75
+  llvm::SmallVector Elems;
+  llvm::SmallVector Offsets;
+  CharUnits Size = CharUnits::Zero();

rjmccall wrote:
> Are there invariants about these?  I assume they're parallel arrays; are they 
> kept sorted?
Added comments to explain.



Comment at: lib/CodeGen/CGExprConstant.cpp:76
+  llvm::SmallVector Offsets;
+  CharUnits Size = CharUnits::Zero();
+

rjmccall wrote:
> This is one past the last byte that's been covered by an actual `Constant*` 
> value, or does it include unoccupied padding, or does it exclude even 
> occupied padding?
Added comment to explain.



Comment at: lib/CodeGen/CGExprConstant.cpp:98
+Offsets.reserve(N);
+  }
+

rjmccall wrote:
> Might be worth clarifying what `N` is here.
Looks like this ended up being unused; removed.



Comment at: lib/CodeGen/CGExprConstant.cpp:190
+bool ConstantBuilder::addBits(llvm::APInt Bits, uint64_t OffsetInBits,
+  bool AllowOverwrite) {
+  const ASTContext  = CGM.getContext();

rjmccall wrote:
> `AllowOversized` (which you used in the interface) seems like a better name.
`AllowOversized` is used to mean "the size of the constant may be larger than 
the size of the type", and is a parameter to `build` / `buildFrom`.
`AllowOverwrite` is used to mean "adding this constant may overwrite something 
you've already been given", and is a parameter to `add` / `addBits`.

I can make these names more different from each other if that would help?



Comment at: lib/CodeGen/CGExprConstant.cpp:258
+  return false;
+assert(CI->getBitWidth() == CharWidth && "splitAt failed");
+assert((!(CI->getValue() & UpdateMask) || AllowOverwrite) &&

rjmccall wrote:
> Oh, because we're splitting before and after a single-`CharUnits` range?  
> That seems worthy of a somewhat clearer explanation in the code.
> 
> I guess we could have a non-`ConstantInt` single-byte value.  Unlikely but 
> not impossible. :)
Yes. It's not too hard to craft a testcase where we'd get an explicit `undef` 
here; added handling for that and for all-zeroes constants, which are both 
correctly handled by overwriting the whole byte.



Comment at: lib/CodeGen/CGExprConstant.cpp:375
+
+  // FIXME: We could theoretically split a ConstantInt if the need ever arose.
+

rjmccall wrote:
> Does this not come up all the time with bit-fields?  I guess we emit them in 
> single-`char` chunks, so it wouldn't.  Probably worth a comment.
Done.

We could hit this case for cases such as:

```
union U { int a; int b : 3; };
struct S { U u; };
S s = {(union U){1234}, .u.b = 5};
```

(which `CodeGen` currently rejects with "cannot compile this static initializer 
yet" in C), and splitting the `ConstantInt` would allow us to emit that 
initializer as a constant, but I'm not sure it's worthwhile unless it lets us 
simplify or improve bitfield emission in general. (The above isn't a case that 
C requires us to treat as a constant initializer, so rejecting it is not a 
conformance issue.)

Maybe instead of splitting bitfields into 1-byte chunks like we currently do, 
we should try to combine them into a single `iN`, like `CGRecordLayoutBuilder` 
does. But splitting to `i8` maintains the status quo, which is what I was 
aiming for in this patch.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63371/new/

https://reviews.llvm.org/D63371



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith updated this revision to Diff 205161.
rsmith marked 13 inline comments as done.
rsmith added a comment.

- Address review comments from rjmccall.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63371/new/

https://reviews.llvm.org/D63371

Files:
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGenCXX/designated-init.cpp

Index: test/CodeGenCXX/designated-init.cpp
===
--- /dev/null
+++ test/CodeGenCXX/designated-init.cpp
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -std=c++98 -emit-llvm -o - %s -triple x86_64-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm -o - %s -triple x86_64-linux-gnu | FileCheck %s
+
+struct A { int x, y[3]; };
+struct B { A a; };
+
+// CHECK: @b = global %{{[^ ]*}} { %{{[^ ]*}} { i32 1, [3 x i32] [i32 2, i32 5, i32 4] } }
+B b = {(A){1, 2, 3, 4}, .a.y[1] = 5};
+
+union U {
+  int n;
+  float f;
+};
+struct C {
+  int x;
+  U u[3];
+};
+struct D {
+  C c;
+};
+
+// CHECK: @d1 = {{.*}} { i32 1, [3 x %[[U:.*]]] [%[[U]] { i32 2 }, %[[U]] { i32 5 }, %[[U]] { i32 4 }] }
+D d1 = {(C){1, {{.n=2}, {.f=3}, {.n=4}}}, .c.u[1].n = 5};
+
+// CHECK: @d2 = {{.*}} { i32 1, { %[[U]], float, %[[U]] } { %[[U]] { i32 2 }, float 5.{{0*}}e+00, %[[U]] { i32 4 } } }
+D d2 = {(C){1, 2, 3, 4}, .c.u[1].f = 5};
+
+struct Bitfield {
+  int a : 3;
+  int b : 4;
+  int c : 5;
+};
+struct WithBitfield {
+  int n;
+  Bitfield b;
+};
+// CHECK: @bitfield = {{.*}} { i32 1, { i8, i8, [2 x i8] } { i8 42, i8 2, [2 x i8] undef } }
+WithBitfield bitfield = {1, (Bitfield){2, 3, 4}, .b.b = 5};
+
+struct String {
+  const char buffer[12];
+};
+struct WithString {
+  String str;
+};
+// CHECK: @string = {{.*}} [12 x i8] c"Hello World\00" } }
+WithString string = {(String){"hello world"}, .str.buffer[0] = 'H', .str.buffer[6] = 'W'};
+
+struct LargeArray {
+  int arr[4096];
+};
+struct WithLargeArray {
+  LargeArray arr;
+};
+// CHECK: @large = global { { <{ [11 x i32], [4085 x i32] }> } } { { <{ [11 x i32], [4085 x i32] }> } { <{ [11 x i32], [4085 x i32] }> <{ [11 x i32] [i32 1, i32 2, i32 3, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 10], [4085 x i32] zeroinitializer }> } }
+WithLargeArray large = {(LargeArray){1, 2, 3}, .arr.arr[10] = 10};
+
+union OverwritePaddingWithBitfield {
+  struct Padding { unsigned : 8; char c; } padding;
+  char bitfield : 3;
+};
+struct WithOverwritePaddingWithBitfield {
+  OverwritePaddingWithBitfield a;
+};
+// CHECK: @overwrite_padding = global { { i8, i8 } } { { i8, i8 } { i8 3, i8 1 } }
+WithOverwritePaddingWithBitfield overwrite_padding = {(OverwritePaddingWithBitfield){1}, .a.bitfield = 3};
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -22,6 +22,8 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/Builtins.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Function.h"
@@ -30,148 +32,583 @@
 using namespace CodeGen;
 
 //===--===//
-//ConstStructBuilder
+//ConstantAggregateBuilder
 //===--===//
 
 namespace {
 class ConstExprEmitter;
+
+struct ConstantAggregateBuilderUtils {
+  CodeGenModule 
+
+  ConstantAggregateBuilderUtils(CodeGenModule ) : CGM(CGM) {}
+
+  CharUnits getAlignment(const llvm::Constant *C) const {
+return CharUnits::fromQuantity(
+CGM.getDataLayout().getABITypeAlignment(C->getType()));
+  }
+
+  CharUnits getSize(llvm::Type *Ty) const {
+return CharUnits::fromQuantity(CGM.getDataLayout().getTypeAllocSize(Ty));
+  }
+
+  CharUnits getSize(const llvm::Constant *C) const {
+return getSize(C->getType());
+  }
+
+  llvm::Constant *getPadding(CharUnits PadSize) const {
+llvm::Type *Ty = CGM.Int8Ty;
+if (PadSize > CharUnits::One())
+  Ty = llvm::ArrayType::get(Ty, PadSize.getQuantity());
+return llvm::UndefValue::get(Ty);
+  }
+
+  llvm::Constant *getZeroes(CharUnits ZeroSize) const {
+llvm::Type *Ty = llvm::ArrayType::get(CGM.Int8Ty, ZeroSize.getQuantity());
+return llvm::ConstantAggregateZero::get(Ty);
+  }
+};
+
+/// Incremental builder for an llvm::Constant* holding a struct or array
+/// constant.
+class ConstantAggregateBuilder : private ConstantAggregateBuilderUtils {
+  /// The elements of the constant. These two arrays must have the same size;
+  /// Offsets[i] describes the offset of Elems[i] within the constant. The
+  /// elements are kept in increasing offset order, and we ensure that there
+  /// is no overlap: Offsets[i+1] >= Offsets[i] + getSize(Elemes[i]).
+  ///
+  /// This may contain explicit padding elements (in order to create a
+  /// 

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGExprConstant.cpp:73
+/// Incremental builder for an llvm::Constant* holding a structure constant.
+class ConstantBuilder : private ConstantBuilderUtils {
+  llvm::SmallVector Elems;

This seems like a very generic name for this type.



Comment at: lib/CodeGen/CGExprConstant.cpp:75
+  llvm::SmallVector Elems;
+  llvm::SmallVector Offsets;
+  CharUnits Size = CharUnits::Zero();

Are there invariants about these?  I assume they're parallel arrays; are they 
kept sorted?



Comment at: lib/CodeGen/CGExprConstant.cpp:76
+  llvm::SmallVector Offsets;
+  CharUnits Size = CharUnits::Zero();
+

This is one past the last byte that's been covered by an actual `Constant*` 
value, or does it include unoccupied padding, or does it exclude even occupied 
padding?



Comment at: lib/CodeGen/CGExprConstant.cpp:98
+Offsets.reserve(N);
+  }
+

Might be worth clarifying what `N` is here.



Comment at: lib/CodeGen/CGExprConstant.cpp:190
+bool ConstantBuilder::addBits(llvm::APInt Bits, uint64_t OffsetInBits,
+  bool AllowOverwrite) {
+  const ASTContext  = CGM.getContext();

`AllowOversized` (which you used in the interface) seems like a better name.



Comment at: lib/CodeGen/CGExprConstant.cpp:196
+  // current char.
+  unsigned CharOffset = OffsetInBits % CharWidth;
+

`OffsetWithinChar`?



Comment at: lib/CodeGen/CGExprConstant.cpp:201
+  for (CharUnits Char = Context.toCharUnitsFromBits(OffsetInBits);
+   /**/; ++Char) {
+// Number of bits we want to fill in this byte.

`OffsetInChars`?



Comment at: lib/CodeGen/CGExprConstant.cpp:237
+  if (!LastElemToUpdate)
+return false;
+  assert(*LastElemToUpdate - *FirstElemToUpdate < 2 &&

Especially in the context of the comment above, I think it would be good to 
clarify that both of these are hard "we can't emit this constant" bail-outs.



Comment at: lib/CodeGen/CGExprConstant.cpp:258
+  return false;
+assert(CI->getBitWidth() == CharWidth && "splitAt failed");
+assert((!(CI->getValue() & UpdateMask) || AllowOverwrite) &&

Oh, because we're splitting before and after a single-`CharUnits` range?  That 
seems worthy of a somewhat clearer explanation in the code.

I guess we could have a non-`ConstantInt` single-byte value.  Unlikely but not 
impossible. :)



Comment at: lib/CodeGen/CGExprConstant.cpp:375
+
+  // FIXME: We could theoretically split a ConstantInt if the need ever arose.
+

Does this not come up all the time with bit-fields?  I guess we emit them in 
single-`char` chunks, so it wouldn't.  Probably worth a comment.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63371/new/

https://reviews.llvm.org/D63371



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D63371#1546587 , @rsmith wrote:

> In D63371#1546500 , @rjmccall wrote:
>
> > Isn't `[[no_unique_address]]` only significant for empty members?  I'm not 
> > sure why they need significant support from constant-building, since they 
> > expand to no meaningful initializer.
>
>
> It also permits reuse of tail padding for non-static data members, which is 
> the complexity that this patch is dealing with (in addition to improving and 
> generalizing the support for non-trivial designated initializers).


I see.

>> We have some code we'll hopefully be upstreaming soon that relies on being 
>> able to do things with address-of-position placeholder values; I'm a little 
>> worried that the new structure here doesn't really support them.
> 
> Can you say a bit more about that? (Do you want to be able to emit a constant 
> that denotes a pointer to somewhere else within the same constant being 
> emitted, or something like that?) I think this approach should be strictly 
> more general than what we had before, but perhaps that means it can't be 
> extended in the direction you need?

We need to be able to construct constant initializers for certain fields in 
terms of (among other things) a pointer to the current field.  My concern was 
whether this might mess up the indexing to the current field; but it looks like 
we actually discover the right GEP indices retroactively for these aggregate 
constants (unlike e.g. `ConstantInitBuilder`), so the fact that the GEP indices 
might change as we build the constant is not a problem.  As long as the 
`ConstantEmitter` is being passed around and used to build the individual 
fields appropriately, it should be fine.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63371/new/

https://reviews.llvm.org/D63371



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D63371#1546500 , @rjmccall wrote:

> Isn't `[[no_unique_address]]` only significant for empty members?  I'm not 
> sure why they need significant support from constant-building, since they 
> expand to no meaningful initializer.


It also permits reuse of tail padding for non-static data members, which is the 
complexity that this patch is dealing with (in addition to improving and 
generalizing the support for non-trivial designated initializers).

> We have some code we'll hopefully be upstreaming soon that relies on being 
> able to do things with address-of-position placeholder values; I'm a little 
> worried that the new structure here doesn't really support them.

Can you say a bit more about that? (Do you want to be able to emit a constant 
that denotes a pointer to somewhere else within the same constant being 
emitted, or something like that?) I think this approach should be strictly more 
general than what we had before, but perhaps that means it can't be extended in 
the direction you need?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63371/new/

https://reviews.llvm.org/D63371



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Isn't `[[no_unique_address]]` only significant for empty members?  I'm not sure 
why they need significant support from constant-building, since they expand to 
no meaningful initializer.

We have some code we'll hopefully be upstreaming soon that relies on being able 
to do things with address-of-position placeholder values; I'm a little worried 
that the new structure here doesn't really support them.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63371/new/

https://reviews.llvm.org/D63371



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith created this revision.
rsmith added a reviewer: rjmccall.
Herald added a project: clang.

This adds a ConstantBuilder class that deals with incrementally building
an aggregate constant, including support for overwriting
previously-emitted parts of the aggregate with new values.

This fixes a bunch of cases where we used to be unable to reduce a
DesignatedInitUpdateExpr down to an IR constant, and also lays some
groundwork for emission of class constants with [[no_unique_address]]
members.


Repository:
  rC Clang

https://reviews.llvm.org/D63371

Files:
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGenCXX/designated-init.cpp

Index: test/CodeGenCXX/designated-init.cpp
===
--- /dev/null
+++ test/CodeGenCXX/designated-init.cpp
@@ -0,0 +1,56 @@
+// RUN: %clang_cc1 -std=c++98 -emit-llvm -o - %s -triple x86_64-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm -o - %s -triple x86_64-linux-gnu | FileCheck %s
+
+struct A { int x, y[3]; };
+struct B { A a; };
+
+// CHECK: @b = global %{{[^ ]*}} { %{{[^ ]*}} { i32 1, [3 x i32] [i32 2, i32 5, i32 4] } }
+B b = {(A){1, 2, 3, 4}, .a.y[1] = 5};
+
+union U {
+  int n;
+  float f;
+};
+struct C {
+  int x;
+  U u[3];
+};
+struct D {
+  C c;
+};
+
+// CHECK: @d1 = {{.*}} { i32 1, [3 x %[[U:.*]]] [%[[U]] { i32 2 }, %[[U]] { i32 5 }, %[[U]] { i32 4 }] }
+D d1 = {(C){1, {{.n=2}, {.f=3}, {.n=4}}}, .c.u[1].n = 5};
+
+// CHECK: @d2 = {{.*}} { i32 1, { %[[U]], float, %[[U]] } { %[[U]] { i32 2 }, float 5.{{0*}}e+00, %[[U]] { i32 4 } } }
+D d2 = {(C){1, 2, 3, 4}, .c.u[1].f = 5};
+
+struct Bitfield {
+  int a : 3;
+  int b : 4;
+  int c : 5;
+};
+struct WithBitfield {
+  int n;
+  Bitfield b;
+};
+// CHECK: @bitfield = {{.*}} { i32 1, { i8, i8, [2 x i8] } { i8 42, i8 2, [2 x i8] undef } }
+WithBitfield bitfield = {1, (Bitfield){2, 3, 4}, .b.b = 5};
+
+struct String {
+  const char buffer[12];
+};
+struct WithString {
+  String str;
+};
+// CHECK: @string = {{.*}} [12 x i8] c"Hello World\00" } }
+WithString string = {(String){"hello world"}, .str.buffer[0] = 'H', .str.buffer[6] = 'W'};
+
+struct LargeArray {
+  int arr[4096];
+};
+struct WithLargeArray {
+  LargeArray arr;
+};
+// CHECK: @large = global { { <{ [11 x i32], [4085 x i32] }> } } { { <{ [11 x i32], [4085 x i32] }> } { <{ [11 x i32], [4085 x i32] }> <{ [11 x i32] [i32 1, i32 2, i32 3, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 10], [4085 x i32] zeroinitializer }> } }
+WithLargeArray large = {(LargeArray){1, 2, 3}, .arr.arr[10] = 10};
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -22,6 +22,8 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/Builtins.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Function.h"
@@ -30,148 +32,559 @@
 using namespace CodeGen;
 
 //===--===//
-//ConstStructBuilder
+//ConstantBuilder
 //===--===//
 
 namespace {
 class ConstExprEmitter;
+
+struct ConstantBuilderUtils {
+  CodeGenModule 
+
+  ConstantBuilderUtils(CodeGenModule ) : CGM(CGM) {}
+
+  CharUnits getAlignment(const llvm::Constant *C) const {
+return CharUnits::fromQuantity(
+CGM.getDataLayout().getABITypeAlignment(C->getType()));
+  }
+
+  CharUnits getSize(llvm::Type *Ty) const {
+return CharUnits::fromQuantity(CGM.getDataLayout().getTypeAllocSize(Ty));
+  }
+
+  CharUnits getSize(const llvm::Constant *C) const {
+return getSize(C->getType());
+  }
+
+  llvm::Constant *getPadding(CharUnits PadSize) const {
+llvm::Type *Ty = CGM.Int8Ty;
+if (PadSize > CharUnits::One())
+  Ty = llvm::ArrayType::get(Ty, PadSize.getQuantity());
+return llvm::UndefValue::get(Ty);
+  }
+
+  llvm::Constant *getZeroes(CharUnits ZeroSize) const {
+llvm::Type *Ty = llvm::ArrayType::get(CGM.Int8Ty, ZeroSize.getQuantity());
+return llvm::ConstantAggregateZero::get(Ty);
+  }
+};
+
+/// Incremental builder for an llvm::Constant* holding a structure constant.
+class ConstantBuilder : private ConstantBuilderUtils {
+  llvm::SmallVector Elems;
+  llvm::SmallVector Offsets;
+  CharUnits Size = CharUnits::Zero();
+
+  /// This is true only if laying out Elems in order as the elements of a
+  /// non-packed LLVM struct will give the correct layout.
+  bool NaturalLayout = true;
+
+  bool split(size_t Index, CharUnits Hint);
+  Optional splitAt(CharUnits Pos);
+
+  static llvm::Constant *buildFrom(CodeGenModule ,
+   ArrayRef Elems,
+   ArrayRef Offsets,
+   CharUnits