X86: Remove UsesMSVCFloatingPoint from MachineModuleInfo

[arsenm]

Summary

This is x86 specific and only used in the AsmPrinter module pass. I
think implementing this by looking at the underlying IR types instead
of the selected instructions is a pretty horrifying implementation,
but it's still available in the AsmPrinter.

[rnk]

This is sort of reasonable, but I feel like people might disagree so I won't go ahead and stamp it.

I feel like the existing code adheres to the philosophy that the IR will not be available after codegen, and that ISel is responsible for looking at the IR and saving all these analysis results. There's something undesirable about looking back at the IR once we reach the end of assembly emission.

On the other hand, it's also nice to not have all these long-lived predicates stashed in a side table that we then have to serialize through MIR.

I think in the end, we should land this change, but I'd like to see another +1. If people don't like this, maybe we can use X86MachineFunctionInfo.

Lastly, has anyone checked if MSVC emits _fltused on AArch64 / ARM? Maybe it's not as target-specific as it appears. +@mstorsjo

[arsenm]

In D123933#3457324, @rnk wrote:

This is sort of reasonable, but I feel like people might disagree so I won't go ahead and stamp it.

I feel like the existing code adheres to the philosophy that the IR will not be available after codegen, and that ISel is responsible for looking at the IR and saving all these analysis results. There's something undesirable about looking back at the IR once we reach the end of assembly emission.

Right, but we're nowhere near that goal. There are tons of backlinks to the IR and targets looking directly at the underlying IR. There are a good number of MachineFunction predicates that look at attributes on the IR for example. Ironically I think this helps move towards the direction of cutting out the IR by making MIR serialization more obviously reliable

[mstorsjo]

In D123933#3457324, @rnk wrote:

Lastly, has anyone checked if MSVC emits _fltused on AArch64 / ARM? Maybe it's not as target-specific as it appears. +@mstorsjo

It does seem like MSVC emits this symbol reference on those architectures too - so I guess technically we should try to emit the same there too. (I haven't heard about issues relating to this being missing, but I don't use MSVC mode as much as mingw mode either.)

llvm/include/llvm/CodeGen/MachineModuleInfo.h

class MachineModuleInfo {
   // synchronous (precise at call-sites only) CFA into .eh_frame. However,
   // even under this switch, we'd like .debug_frame to be precise when using
   // -g. At this moment, there's no way to specify that some CFI directives
   // go into .eh_frame only, while others go into .debug_frame only.
 
   /// True if debugging information is available in this module.
   bool DbgInfoAvailable;
 
-  /// True if this module is being built for windows/msvc, and uses floating
-  /// point.  This is used to emit an undefined reference to _fltused.
-  bool UsesMSVCFloatingPoint;
-
   /// True if the module calls the __morestack function indirectly, as is
   /// required under the large code model on x86. This is used to emit
   /// a definition of a symbol, __morestack_addr, containing the address. See
   /// comments in lib/Target/X86/X86FrameLowering.cpp for more details.
   bool UsesMorestackAddr;
 
   /// True if the module contains split-stack functions. This is used to
   /// emit .note.GNU-split-stack section as required by the linker for
public:
   const Ty &getObjFileInfo() const {
     return const_cast<MachineModuleInfo*>(this)->getObjFileInfo<Ty>();
   }
 
   /// Returns true if valid debug info is present.
   bool hasDebugInfo() const { return DbgInfoAvailable; }
   void setDebugInfoAvailability(bool avail) { DbgInfoAvailable = avail; }
 
-  bool usesMSVCFloatingPoint() const { return UsesMSVCFloatingPoint; }
-
-  void setUsesMSVCFloatingPoint(bool b) { UsesMSVCFloatingPoint = b; }
-
   bool usesMorestackAddr() const {
     return UsesMorestackAddr;
   }
 
   void setUsesMorestackAddr(bool b) {
     UsesMorestackAddr = b;
   }
 

llvm/lib/CodeGen/MachineModuleInfo.cpp

 void MMIAddrLabelMapCallbackPtr::allUsesReplacedWith(Value *V2) {
   Map->UpdateForRAUWBlock(cast<BasicBlock>(getValPtr()), cast<BasicBlock>(V2));
 }
 
 void MachineModuleInfo::initialize() {
   ObjFileMMI = nullptr;
   CurCallSite = 0;
   NextFnNum = 0;
-  UsesMSVCFloatingPoint = UsesMorestackAddr = false;
+  UsesMorestackAddr = false;
   HasSplitStack = HasNosplitStack = false;
   AddrLabelSymbols = nullptr;
 }
 
 void MachineModuleInfo::finalize() {
   Personalities.clear();
 
   delete AddrLabelSymbols;
MachineModuleInfo::MachineModuleInfo(MachineModuleInfo &&MMI)
     : TM(std::move(MMI.TM)),
       Context(MMI.TM.getTargetTriple(), MMI.TM.getMCAsmInfo(),
               MMI.TM.getMCRegisterInfo(), MMI.TM.getMCSubtargetInfo(), nullptr,
               nullptr, false),
       MachineFunctions(std::move(MMI.MachineFunctions)) {
   Context.setObjectFileInfo(MMI.TM.getObjFileLowering());
   ObjFileMMI = MMI.ObjFileMMI;
   CurCallSite = MMI.CurCallSite;
-  UsesMSVCFloatingPoint = MMI.UsesMSVCFloatingPoint;
   UsesMorestackAddr = MMI.UsesMorestackAddr;
   HasSplitStack = MMI.HasSplitStack;
   HasNosplitStack = MMI.HasNosplitStack;
   AddrLabelSymbols = MMI.AddrLabelSymbols;
   ExternalContext = MMI.ExternalContext;
   TheModule = MMI.TheModule;
 }
 

llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

for (BasicBlock::iterator I = BB.begin(); (PN = dyn_cast<PHINode>(I)); ++I)
         SplitCriticalEdge(
             Pred->getTerminator(), GetSuccessorNumber(Pred, &BB),
             CriticalEdgeSplittingOptions(DT, LI).setMergeIdenticalEdges());
         goto ReprocessBlock;
       }
   }
 }
 
-static void computeUsesMSVCFloatingPoint(const Triple &TT, const Function &F,
-                                         MachineModuleInfo &MMI) {
-  // Only needed for MSVC
-  if (!TT.isWindowsMSVCEnvironment())
-    return;
-
-  // If it's already set, nothing to do.
-  if (MMI.usesMSVCFloatingPoint())
-    return;
-
-  for (const Instruction &I : instructions(F)) {
-    if (I.getType()->isFPOrFPVectorTy()) {
-      MMI.setUsesMSVCFloatingPoint(true);
-      return;
-    }
-    for (const auto &Op : I.operands()) {
-      if (Op->getType()->isFPOrFPVectorTy()) {
-        MMI.setUsesMSVCFloatingPoint(true);
-        return;
-      }
-    }
-  }
-}
-
 bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf) {
   // If we already selected that function, we do not need to run SDISel.
   if (mf.getProperties().hasProperty(
           MachineFunctionProperties::Property::Selected))
     return false;
   // Do some sanity-checking on the command-line options.
   assert((!EnableFastISelAbort || TM.Options.EnableFastISel) &&
          "-fast-isel-abort > 0 requires -fast-isel");
for (const auto &MI : MBB) {
         MF->setHasInlineAsm(true);
       }
     }
   }
 
   // Determine if there is a call to setjmp in the machine function.
   MF->setExposesReturnsTwice(Fn.callsFunctionThatReturnsTwice());
 
-  // Determine if floating point is used for msvc
-  computeUsesMSVCFloatingPoint(TM.getTargetTriple(), Fn, MF->getMMI());
-
   // Release function-specific state. SDB and CurDAG are already cleared
   // at this point.
   FuncInfo->clear();
 
   LLVM_DEBUG(dbgs() << "*** MachineFunction at end of ISel ***\n");
   LLVM_DEBUG(MF->print(dbgs()));
 
   return true;

llvm/lib/Target/X86/X86AsmPrinter.cpp

 #include "X86Subtarget.h"
 #include "llvm/BinaryFormat/COFF.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/CodeGen/MachineConstantPool.h"
 #include "llvm/CodeGen/MachineModuleInfoImpls.h"
 #include "llvm/CodeGen/TargetLoweringObjectFileImpl.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/InstIterator.h"
 #include "llvm/IR/Mangler.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Type.h"
 #include "llvm/MC/MCCodeEmitter.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCSectionCOFF.h"
 #include "llvm/MC/MCSectionELF.h"
if (!Stubs.empty()) {
     for (auto &Stub : Stubs)
       emitNonLazySymbolPointer(OutStreamer, Stub.first, Stub.second);
 
     Stubs.clear();
     OutStreamer.AddBlankLine();
   }
 }
 
+/// True if this module is being built for windows/msvc, and uses floating
+/// point.  This is used to emit an undefined reference to _fltused.
+static bool usesMSVCFloatingPoint(const Triple &TT, const Module &M) {
+  // Only needed for MSVC
+  if (!TT.isWindowsMSVCEnvironment())
+    return false;
+
+  for (const Function &F : M) {
+    for (const Instruction &I : instructions(F)) {
+      if (I.getType()->isFPOrFPVectorTy())
+        return true;
+
+      for (const auto &Op : I.operands()) {
+        if (Op->getType()->isFPOrFPVectorTy())
+          return true;
+      }
+    }
+  }
+
+  return false;
+}
+
 void X86AsmPrinter::emitEndOfAsmFile(Module &M) {
   const Triple &TT = TM.getTargetTriple();
 
   if (TT.isOSBinFormatMachO()) {
     // Mach-O uses non-lazy symbol stubs to encode per-TU information into
     // global table for symbol lookup.
     emitNonLazyStubs(MMI, *OutStreamer);
 
     // Emit stack and fault map information.
     emitStackMaps(SM);
     FM.serializeToFaultMapSection();
 
     // This flag tells the linker that no global symbols contain code that fall
     // through to other global symbols (e.g. an implementation of multiple entry
     // points). If this doesn't occur, the linker can safely perform dead code
     // stripping. Since LLVM never generates code that does this, it is always
     // safe to set.
     OutStreamer->emitAssemblerFlag(MCAF_SubsectionsViaSymbols);
   } else if (TT.isOSBinFormatCOFF()) {
-    if (MMI->usesMSVCFloatingPoint()) {
+    if (usesMSVCFloatingPoint(TT, M)) {
       // In Windows' libcmt.lib, there is a file which is linked in only if the
       // symbol _fltused is referenced. Linking this in causes some
       // side-effects:
       //
       // 1. For x86-32, it will set the x87 rounding mode to 53-bit instead of
       // 64-bit mantissas at program start.
       //
       // 2. It links in support routines for floating-point in scanf and printf.