[CodeGen] Sort .ctors in reverse on MinGW just like on other platforms

[mstorsjo]

On MinGW targets, the .ctors section is always used (as opposed to on ELF platforms, where .init_section is the default but one can select using .ctors, with the -use-ctors option, or the UseInitArray field in TargetOptions).

Apply the reverse ordering regardless of whether the caller has set the UseInitArray flag for this target, as this target unconditionally uses the .ctors section anyway.

For the CodeGen/X86/constructor.ll testcase, note how this now produces the same output ordering as for ELF targets with -use-ctors.

This fixes #55938.

[llvmbot]

@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-mc

@llvm/pr-subscribers-platform-windows

Changes

On MinGW targets, the .ctors section is always used (as opposed to on ELF platforms, where .init_section is the default but one can select using .ctors, with the -use-ctors option, or the UseInitArray field in TargetOptions).

Apply the reverse ordering regardless of whether the caller has set the UseInitArray flag for this target, as this target unconditionally uses the .ctors section anyway.

For the CodeGen/X86/constructor.ll testcase, note how this now produces the same output ordering as for ELF targets with -use-ctors.

This fixes #55938.

---

Full diff: https://github.com/llvm/llvm-project/pull/68570.diff

3 Files Affected:

• (modified) llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (+5-1)
• (modified) llvm/test/CodeGen/X86/constructor.ll (+6-6)
• (modified) llvm/test/MC/COFF/global_ctors_dtors.ll (+2-2)

diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 97d2fe3426406f3..5f4141e17541bf7 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -2807,7 +2807,11 @@ void AsmPrinter::emitXXStructorList(const DataLayout &DL, const Constant *List,
 
   // Emit the structors in reverse order if we are using the .ctor/.dtor
   // initialization scheme.
-  if (!TM.Options.UseInitArray)
+  bool UseCtorSection = !TM.Options.UseInitArray;
+  // MinGW targets always use the .ctors section.
+  if (TM.getTargetTriple().isWindowsGNUEnvironment())
+    UseCtorSection = true;
+  if (UseCtorSection)
     std::reverse(Structors.begin(), Structors.end());
 
   const Align Align = DL.getPointerPrefAlignment();
diff --git a/llvm/test/CodeGen/X86/constructor.ll b/llvm/test/CodeGen/X86/constructor.ll
index 0fea69b5a7bcb52..4bc4e72b6472a68 100644
--- a/llvm/test/CodeGen/X86/constructor.ll
+++ b/llvm/test/CodeGen/X86/constructor.ll
@@ -77,14 +77,14 @@ entry:
 ; MCU-CTORS:         .section        .ctors,"aw",@progbits
 ; MCU-INIT-ARRAY:    .section        .init_array,"aw",@init_array
 
-; COFF-CTOR:		.section	.ctors.65520,"dw",associative,v
+; COFF-CTOR:		.section	.ctors,"dw"
 ; COFF-CTOR-NEXT:	.p2align	3
-; COFF-CTOR-NEXT:	.quad	g
+; COFF-CTOR-NEXT:	.quad	j
+; COFF-CTOR-NEXT:	.quad	i
+; COFF-CTOR-NEXT:	.quad	f
 ; COFF-CTOR-NEXT:	.section	.ctors.09980,"dw",associative,v
 ; COFF-CTOR-NEXT:	.p2align	3
 ; COFF-CTOR-NEXT:	.quad	h
-; COFF-CTOR-NEXT:	.section	.ctors,"dw"
+; COFF-CTOR-NEXT:	.section	.ctors.65520,"dw",associative,v
 ; COFF-CTOR-NEXT:	.p2align	3
-; COFF-CTOR-NEXT:	.quad	f
-; COFF-CTOR-NEXT:	.quad	i
-; COFF-CTOR-NEXT:	.quad	j
+; COFF-CTOR-NEXT:	.quad	g
diff --git a/llvm/test/MC/COFF/global_ctors_dtors.ll b/llvm/test/MC/COFF/global_ctors_dtors.ll
index 7df4d3e500b54c3..9b321a082a32e58 100644
--- a/llvm/test/MC/COFF/global_ctors_dtors.ll
+++ b/llvm/test/MC/COFF/global_ctors_dtors.ll
@@ -56,10 +56,10 @@ define i32 @main() nounwind {
 ; WIN32-NOT: c_global_ctor
 ; WIN32: .section .CRT$XTX,"dr"
 ; WIN32: a_global_dtor
-; MINGW32: .section .ctors,"dw"
-; MINGW32: a_global_ctor
 ; MINGW32: .section .ctors,"dw",associative,{{_?}}b
 ; MINGW32: b_global_ctor
 ; MINGW32-NOT: c_global_ctor
+; MINGW32: .section .ctors,"dw"
+; MINGW32: a_global_ctor
 ; MINGW32: .section .dtors,"dw"
 ; MINGW32: a_global_dtor

[alvinhochun]

Not quite sure if putting a target-specific special case in emitXXStructorList is really the best way to handle it. The condition also doesn't exactly match what TargetLoweringObjectFileCOFF checks for:

llvm-project/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

Line 1928 in f74e9f8

if (T.isWindowsMSVCEnvironment() || T.isWindowsItaniumEnvironment()) {

Are there any other targets that may need the fix? I guess Cygwin could but I don't know if LLVM even works for this target.

[mstorsjo]

The condition also doesn't exactly match what TargetLoweringObjectFileCOFF checks for:

llvm-project/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

Line 1928 in f74e9f8

if (T.isWindowsMSVCEnvironment() || T.isWindowsItaniumEnvironment()) {

Are there any other targets that may need the fix? I guess Cygwin could but I don't know if LLVM even works for this target.

That's true, I guess it should be done for Cygwin as well; and matching the exact condition in TargetLoweringObjectFileCOFF would certainly be best.

Cygwin is not at all as well tested as MinGW, but there's been some recent effort on this front from @carlo-bramini, so I believe it might be working at least reasonably (but I'm fairly sure that there's lots of such corner case details that might need to be extended from isWindowsGNUEnvironment() into isOSCygMing()).

Not quite sure if putting a target-specific special case in emitXXStructorList is really the best way to handle it.

See the discussion from @efriedma-quic in #68571 for other ways to improve this. I guess that would boil down to

1. Extend the UseInitArray param to allow for a Default state
2. Hardcode the logic here for cases where we know .ctors is the only choice
3. Error out if the option is set in a conflicting way (e.g. UseInitArray set to explicit true on a MinGW target).

[carlo-bramini]

That's true, I guess it should be done for Cygwin as well;

I also think that it should be done for CYGWIN, if it fixes that issue.

[MaskRay]

It's better to make UseInitArray (set by -fno-use-init-array in clang/lib/Driver/ToolChains/*.cpp) actually reflect .ctors/.init_array. For llc's default, you can update codegen::InitTargetOptionsFromCodeGenFlags.

Note that having llc test coverage in llvm/test/CodeGen/X86/ does not test clangCodeGen, but this test gap is what we have for these TargetOptions and it's probably fine.