[llvm-branch-commits] [llvm] CodeGen: Remove UsesMSVCFloatingPoint from MachineModuleInfo (PR #100368)
Matt Arsenault via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Jul 25 23:37:14 PDT 2024
https://github.com/arsenm updated https://github.com/llvm/llvm-project/pull/100368
>From db429d7de96c0b5c80b015adc73a13025f93d4ad Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Sun, 17 Apr 2022 10:28:14 -0400
Subject: [PATCH 1/2] CodeGen: Remove UsesMSVCFloatingPoint from
MachineModuleInfo
This is only used by x86 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.
This is https://reviews.llvm.org/D123933 resurrected.
I still don't know what the point of emitting _fltused is, but this
approach of looking at the IR types probably isn't the right way to
do this in the first place. If the intent is report any FP instructions,
this will miss any implicitly introduced ones during codegen. Also don't
know why just unconditionally emitting it isn't an option.
The last review mentioned the ARMs might want to emit this, but I'm
not going to go fix that. If someone wants to emit this on ARM, they
can move this to a common helper or analysis somewhere.
---
llvm/include/llvm/CodeGen/MachineModuleInfo.h | 8 ------
llvm/lib/CodeGen/MachineModuleInfo.cpp | 1 -
.../CodeGen/SelectionDAG/SelectionDAGISel.cpp | 27 -------------------
llvm/lib/Target/X86/X86AsmPrinter.cpp | 25 ++++++++++++++++-
4 files changed, 24 insertions(+), 37 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/MachineModuleInfo.h b/llvm/include/llvm/CodeGen/MachineModuleInfo.h
index 97b439c726b0a..dfa0e993ec06a 100644
--- a/llvm/include/llvm/CodeGen/MachineModuleInfo.h
+++ b/llvm/include/llvm/CodeGen/MachineModuleInfo.h
@@ -116,10 +116,6 @@ class MachineModuleInfo {
/// True if debugging information is available in this module.
bool DbgInfoAvailable = false;
- /// 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 = false;
-
/// Maps IR Functions to their corresponding MachineFunctions.
DenseMap<const Function*, std::unique_ptr<MachineFunction>> MachineFunctions;
/// Next unique number available for a MachineFunction.
@@ -189,10 +185,6 @@ class MachineModuleInfo {
/// Returns true if valid debug info is present.
bool hasDebugInfo() const { return DbgInfoAvailable; }
- bool usesMSVCFloatingPoint() const { return UsesMSVCFloatingPoint; }
-
- void setUsesMSVCFloatingPoint(bool b) { UsesMSVCFloatingPoint = b; }
-
/// \name Exception Handling
/// \{
diff --git a/llvm/lib/CodeGen/MachineModuleInfo.cpp b/llvm/lib/CodeGen/MachineModuleInfo.cpp
index 088e76029f1a3..150ab363c8fcd 100644
--- a/llvm/lib/CodeGen/MachineModuleInfo.cpp
+++ b/llvm/lib/CodeGen/MachineModuleInfo.cpp
@@ -28,7 +28,6 @@ void MachineModuleInfo::initialize() {
ObjFileMMI = nullptr;
CurCallSite = 0;
NextFnNum = 0;
- UsesMSVCFloatingPoint = false;
DbgInfoAvailable = false;
}
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 401d23b22adcd..84331d257a3d0 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -417,30 +417,6 @@ void SelectionDAGISelLegacy::getAnalysisUsage(AnalysisUsage &AU) const {
MachineFunctionPass::getAnalysisUsage(AU);
}
-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;
- }
- }
- }
-}
-
PreservedAnalyses
SelectionDAGISelPass::run(MachineFunction &MF,
MachineFunctionAnalysisManager &MFAM) {
@@ -802,9 +778,6 @@ bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf) {
}
}
- // Determine if floating point is used for msvc
- computeUsesMSVCFloatingPoint(TM.getTargetTriple(), Fn, *CurDAG->getMMI());
-
// Release function-specific state. SDB and CurDAG are already cleared
// at this point.
FuncInfo->clear();
diff --git a/llvm/lib/Target/X86/X86AsmPrinter.cpp b/llvm/lib/Target/X86/X86AsmPrinter.cpp
index 0c2c6bf7f8b70..9d86a9c9d1609 100644
--- a/llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ b/llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -28,6 +28,7 @@
#include "llvm/CodeGenTypes/MachineValueType.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"
@@ -975,6 +976,28 @@ static void emitNonLazyStubs(MachineModuleInfo *MMI, MCStreamer &OutStreamer) {
}
}
+/// 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();
@@ -993,7 +1016,7 @@ void X86AsmPrinter::emitEndOfAsmFile(Module &M) {
// 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:
>From 9b8483a9f4ea411f90745aa392c24b3aebe789e1 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Wed, 24 Jul 2024 22:25:27 +0400
Subject: [PATCH 2/2] Add comment
---
llvm/lib/Target/X86/X86AsmPrinter.cpp | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Target/X86/X86AsmPrinter.cpp b/llvm/lib/Target/X86/X86AsmPrinter.cpp
index 9d86a9c9d1609..957eb21cf4f8c 100644
--- a/llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ b/llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -977,7 +977,12 @@ static void emitNonLazyStubs(MachineModuleInfo *MMI, MCStreamer &OutStreamer) {
}
/// True if this module is being built for windows/msvc, and uses floating
-/// point. This is used to emit an undefined reference to _fltused.
+/// point. This is used to emit an undefined reference to _fltused. This is
+/// needed in Windows kernel or driver contexts to find and prevent code from
+/// modifying non-GPR registers.
+///
+/// TODO: It would be better if this was computed from MIR by looking for
+/// selected floating-point instructions.
static bool usesMSVCFloatingPoint(const Triple &TT, const Module &M) {
// Only needed for MSVC
if (!TT.isWindowsMSVCEnvironment())
More information about the llvm-branch-commits
mailing list