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

Mon Jun 17 11:44:29 PDT 2019

rjmccall added a comment.

In D63371#1546587 <https://reviews.llvm.org/D63371#1546587>, @rsmith wrote:

> In D63371#1546500 <https://reviews.llvm.org/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