[PATCH] D71360: Honor -fuse-init-array when os is not specified on x86

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

It looks to me as if essentially all ELF targets want to honor the UseInitArray target option:

  llvm/lib/Target/AArch64/AArch64TargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
  llvm/lib/Target/Hexagon/HexagonTargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
  llvm/lib/Target/Lanai/LanaiTargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
  llvm/lib/Target/Mips/MipsTargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
  llvm/lib/Target/PowerPC/PPCTargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
  llvm/lib/Target/RISCV/RISCVTargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
  llvm/lib/Target/Sparc/SparcTargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
  llvm/lib/Target/X86/X86TargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
  llvm/lib/Target/X86/X86TargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
  llvm/lib/Target/X86/X86TargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
  llvm/lib/Target/X86/X86TargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);

I think the code would be cleaner if we:

- hoisted that setting up into some shared base class
- OSs that insist on using ctors and not honoring this flag should be specialized to disable it (maybe NetBSD from your patch?)

What's needed here to prevent this type of problem in the future is a cleanup to reduce needlessly duplicated functionality, not yet another special case for OS-less triples.



================
Comment at: llvm/lib/Target/X86/X86TargetObjectFile.h:52
 
+  /// X86BinFormatELFTargetObjectFile - This implementation is used for OS
+  /// which uses ELF Binary Format on x86 and x86-64.
----------------
I don't see how this differs in behavior in a meaningful way from X86ELFTargetObjectFile. Can you add the override there without breaking too much?


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

https://reviews.llvm.org/D71360