[PATCH] D103495: [static initializers] Don't put ordered dynamic initializers of static variables into global_ctors

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: asbirlea.
rnk added a comment.

I think we do need to think about globalopt. Consider this example:

  extern "C" {
  int printf(const char *, ...);
  extern int sharedState;
  int sharedState = 0;
  int init1() {
    asm volatile(""); // block globalopt
    return ++sharedState;
  }
  int init2() { return ++sharedState; }
  int gv1 = init1();
  int gv2 = init2();
  }
  
  int main() {
    printf("gv1: %d\n", gv1);
    printf("gv2: %d\n", gv2);
  }

These initializers are ordered, and gv1 should be 1 and gv2 should be 2. Clang produces a single shared initializer `@_GLOBAL__sub_I_${src}` to ensure this ordering. However, if you manually split the initializers apart (https://reviews.llvm.org/P8266) and try to rely on the new langref ordering, globalopt will fold the second initializer and it appears as if `init2` ran first. I have these two files in t.cpp and t.ll, and see the results:

  $ clang -O2 t.cpp  && ./a.out
  gv1: 1
  gv2: 2
  
  $ clang -O2 t.ll  && ./a.out
  gv1: 2
  gv2: 1

I've never been totally comfortable with the soundness of globalopt, so I wonder if that's the part that really needs to change. The value of globalopt is that it can fold simple constructors, and those don't access other mutable global variables. If we had some way of telling globalopt that it is only allowed to read from constants and the GV being initialized, that would solve this issue. Right now, there is no link in the IR between a global and its initializer unless the global is comdat.

---

As for this patch, I think we're not ready to add the LangRef guarantee. However, the code change is good: it ensures that -f[no-]use-init-array doesn't needlessly reverse the order of initialization in the TU.

Without the langref guarantee, clang is skating on thin ice: inline static data member initializers may not be run in source order. However, globalopt is currently completely unable to fold initializers for weak/comdat globals, so in practice, there is no risk to users.

I think we can defer the langref change until the next time someone makes a serious investment in globalopt. I think @asbirlea and @aeubanks might be interested in that sometime later this year.


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

https://reviews.llvm.org/D103495