[PATCH] D32330: X86: Don't emit zero-byte functions on Windows

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1053-1054
+  // https://developercommunity.visualstudio.com/content/problem/45366/vc-linker-creates-invalid-dll-with-clang-cl.html
+  if (!HasAnyRealCode &&
+      (MAI->hasSubsectionsViaSymbols() || TM.getTargetTriple().isOSWindows())) {
     MCInst Noop;
----------------
This whole code feels a bit out of place, as we have macho and windows specific handling as part of the generic AsmPrinter now. Should at least have a `FIXME` or better a new interface to hide this behind some API. So you can query MCAsmInfo or MCTargetStreamer (not sure what is best with my limited MC experience) whether empty functions should be avoided.


================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1056
     MCInst Noop;
+    // FIXME: getNoopForMachoTarget() is actually not just for Mach-O.
     MF->getSubtarget().getInstrInfo()->getNoopForMachoTarget(Noop);
----------------
hans wrote:
> I checked, and there are no in-tree targets that "opt out" in this way.
> It would be awkward, because getNoopForMachoTarget() is also used for other things, e.g. ARMAsmPrinter::EmitSled.
You can just rename as part of this commit instead of adding a FIXME. `getNoopForEmptyFunction()` maybe?


https://reviews.llvm.org/D32330