[PATCH] D109386: Change to use ValueHandle for associated data of GlobalCtors

Wed Sep 8 17:25:18 PDT 2021

rjmccall added a comment.

It shouldn't need a large program to duplicate, but the test will need to exercise some unusual, arguably invalid conditions.

We need to add an entry with associated data to `GlobalCtors` or `GlobalDtors` and then replace the associated data.  We only actually use associated data in either of these lists when we have a C++ global variable with a dynamic initializer that needs a COMDAT key.  So first of all, the target needs to use COMDAT.  Also, the variable needs to satisfy the conditions for using a COMDAT key, which is apparently just being externally visible (which is surprising to me — I would expect this to only be useful on entities with vague linkage?).  If all of that is true, then the COMDAT key we use for these entities is just the address of the variable.

Okay, so the test needs to trigger a replacement of the llvm::GlobalVariable that we emitted a constructor for.  The main reason we replace global variables is that we emitted a use of it with one IR type and then, when we got around to emitting its definition, we realized it needed to have a different IR type.  (This is really just a bad representation in LLVM: external variable declarations don't actually need a value type, and it just causes problems.)  This can happen in several different situations, like if the type of the variable was initially incomplete, or in some cases involving unions.  But none of that should apply here, because we actually compute a constant initializer for the global before we register a dynamic constructor function for it, so I can't see why we'd need to replace it.

My guess is that somehow your program is causing a second variable to be emitted with the same name, maybe using asm labels, or maybe due to some oversight in the mangling rules.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109386