[llvm] Revert "[ms] [llvm-ml] Implement support for PROC NEAR/FAR" (PR #138353)
Eric Astor via llvm-commits
llvm-commits at lists.llvm.org
Fri May 2 15:16:47 PDT 2025
https://github.com/ericastor created https://github.com/llvm/llvm-project/pull/138353
Reverts llvm/llvm-project#131707 - apparently it had gotten into a bad state, and will need relanding.
>From e9b8c1ea6a7a7ead31f7e003408c794df2c8965a Mon Sep 17 00:00:00 2001
From: Eric Astor <epastor at google.com>
Date: Fri, 2 May 2025 18:16:27 -0400
Subject: [PATCH] Revert "[ms] [llvm-ml] Implement support for PROC NEAR/FAR
(#131707)"
This reverts commit 9b4f747827489c5969ff903c728e65c545f58eaa.
---
llvm/include/llvm/MC/MCParser/MCMasmParser.h | 29 -------
llvm/include/llvm/MC/MCSymbolCOFF.h | 4 -
llvm/lib/MC/MCParser/COFFMasmParser.cpp | 80 +++++--------------
llvm/lib/MC/MCParser/MasmParser.cpp | 17 +---
.../lib/Target/X86/AsmParser/X86AsmParser.cpp | 54 +------------
llvm/test/tools/llvm-ml/proc_distance.asm | 56 -------------
6 files changed, 26 insertions(+), 214 deletions(-)
delete mode 100644 llvm/include/llvm/MC/MCParser/MCMasmParser.h
delete mode 100644 llvm/test/tools/llvm-ml/proc_distance.asm
diff --git a/llvm/include/llvm/MC/MCParser/MCMasmParser.h b/llvm/include/llvm/MC/MCParser/MCMasmParser.h
deleted file mode 100644
index a34c6eba8bc59..0000000000000
--- a/llvm/include/llvm/MC/MCParser/MCMasmParser.h
+++ /dev/null
@@ -1,29 +0,0 @@
-//===- llvm/MC/MasmParser.h - MASM Parser Interface -------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_MC_MCPARSER_MCMASMPARSER_H
-#define LLVM_MC_MCPARSER_MCMASMPARSER_H
-
-#include "llvm/MC/MCParser/MCAsmParser.h"
-
-namespace llvm {
-
-/// MASM-type assembler parser interface.
-class MCMasmParser : public MCAsmParser {
-public:
- virtual bool getDefaultRetIsFar() const = 0;
- virtual void setDefaultRetIsFar(bool IsFar) = 0;
-
- bool isParsingMasm() const override { return true; }
-
- static bool classof(const MCAsmParser *AP) { return AP->isParsingMasm(); }
-};
-
-} // end namespace llvm
-
-#endif // LLVM_MC_MCPARSER_MCMASMPARSER_H
diff --git a/llvm/include/llvm/MC/MCSymbolCOFF.h b/llvm/include/llvm/MC/MCSymbolCOFF.h
index c0ddef5cfee50..2964c521e8e44 100644
--- a/llvm/include/llvm/MC/MCSymbolCOFF.h
+++ b/llvm/include/llvm/MC/MCSymbolCOFF.h
@@ -25,7 +25,6 @@ class MCSymbolCOFF : public MCSymbol {
SF_ClassShift = 0,
SF_SafeSEH = 0x0100,
- SF_FarProc = 0x0200,
SF_WeakExternalCharacteristicsMask = 0x0E00,
SF_WeakExternalCharacteristicsShift = 9,
};
@@ -67,9 +66,6 @@ class MCSymbolCOFF : public MCSymbol {
modifyFlags(SF_SafeSEH, SF_SafeSEH);
}
- bool isFarProc() const { return getFlags() & SF_FarProc; }
- void setIsFarProc() const { modifyFlags(SF_FarProc, SF_FarProc); }
-
static bool classof(const MCSymbol *S) { return S->isCOFF(); }
};
diff --git a/llvm/lib/MC/MCParser/COFFMasmParser.cpp b/llvm/lib/MC/MCParser/COFFMasmParser.cpp
index 94f69402ad082..8464a2392680b 100644
--- a/llvm/lib/MC/MCParser/COFFMasmParser.cpp
+++ b/llvm/lib/MC/MCParser/COFFMasmParser.cpp
@@ -12,9 +12,7 @@
#include "llvm/MC/MCAsmMacro.h"
#include "llvm/MC/MCContext.h"
#include "llvm/MC/MCParser/MCAsmLexer.h"
-#include "llvm/MC/MCParser/MCAsmParser.h"
#include "llvm/MC/MCParser/MCAsmParserExtension.h"
-#include "llvm/MC/MCParser/MCMasmParser.h"
#include "llvm/MC/MCParser/MCTargetAsmParser.h"
#include "llvm/MC/MCSectionCOFF.h"
#include "llvm/MC/MCStreamer.h"
@@ -43,7 +41,6 @@ class COFFMasmParser : public MCAsmParserExtension {
StringRef COMDATSymName, COFF::COMDATType Type,
Align Alignment);
- bool parseDirectiveModel(StringRef, SMLoc);
bool parseDirectiveProc(StringRef, SMLoc);
bool parseDirectiveEndProc(StringRef, SMLoc);
bool parseDirectiveSegment(StringRef, SMLoc);
@@ -170,7 +167,7 @@ class COFFMasmParser : public MCAsmParserExtension {
// .exit
// .fardata
// .fardata?
- addDirectiveHandler<&COFFMasmParser::parseDirectiveModel>(".model");
+ addDirectiveHandler<&COFFMasmParser::IgnoreDirective>(".model");
// .stack
// .startup
@@ -204,13 +201,8 @@ class COFFMasmParser : public MCAsmParserExtension {
}
/// Stack of active procedure definitions.
- enum ProcDistance { PROC_DISTANCE_NEAR = 0, PROC_DISTANCE_FAR = 1 };
- struct ProcInfo {
- StringRef Name;
- ProcDistance Distance = PROC_DISTANCE_NEAR;
- bool IsFramed = false;
- };
- SmallVector<ProcInfo, 1> CurrentProcedures;
+ SmallVector<StringRef, 1> CurrentProcedures;
+ SmallVector<bool, 1> CurrentProceduresFramed;
public:
COFFMasmParser() = default;
@@ -443,75 +435,48 @@ bool COFFMasmParser::parseDirectiveOption(StringRef Directive, SMLoc Loc) {
return false;
}
-/// parseDirectiveModel
-/// ::= ".model" "flat"
-bool COFFMasmParser::parseDirectiveModel(StringRef Directive, SMLoc Loc) {
- if (!getLexer().is(AsmToken::Identifier))
- return TokError("expected identifier in directive");
-
- StringRef ModelType = getTok().getIdentifier();
- if (!ModelType.equals_insensitive("flat")) {
- return TokError(
- "expected 'flat' for memory model; no other models supported");
- }
-
- // Ignore; no action necessary.
- Lex();
- return false;
-}
-
/// parseDirectiveProc
/// TODO(epastor): Implement parameters and other attributes.
-/// ::= label "proc" [[distance]] [[frame]]
+/// ::= label "proc" [[distance]]
/// statements
/// label "endproc"
bool COFFMasmParser::parseDirectiveProc(StringRef Directive, SMLoc Loc) {
if (!getStreamer().getCurrentFragment())
return Error(getTok().getLoc(), "expected section directive");
- ProcInfo Proc;
- if (getParser().parseIdentifier(Proc.Name))
+ StringRef Label;
+ if (getParser().parseIdentifier(Label))
return Error(Loc, "expected identifier for procedure");
- while (getLexer().is(AsmToken::Identifier)) {
+ if (getLexer().is(AsmToken::Identifier)) {
StringRef nextVal = getTok().getString();
SMLoc nextLoc = getTok().getLoc();
if (nextVal.equals_insensitive("far")) {
+ // TODO(epastor): Handle far procedure definitions.
Lex();
- Proc.Distance = PROC_DISTANCE_FAR;
- nextVal = getTok().getString();
- nextLoc = getTok().getLoc();
+ return Error(nextLoc, "far procedure definitions not yet supported");
} else if (nextVal.equals_insensitive("near")) {
Lex();
- Proc.Distance = PROC_DISTANCE_NEAR;
- nextVal = getTok().getString();
- nextLoc = getTok().getLoc();
- } else if (nextVal.equals_insensitive("frame")) {
- Lex();
- Proc.IsFramed = true;
nextVal = getTok().getString();
nextLoc = getTok().getLoc();
- } else {
- break;
}
}
- MCSymbolCOFF *Sym =
- cast<MCSymbolCOFF>(getContext().getOrCreateSymbol(Proc.Name));
+ MCSymbolCOFF *Sym = cast<MCSymbolCOFF>(getContext().getOrCreateSymbol(Label));
// Define symbol as simple external function
Sym->setExternal(true);
Sym->setType(COFF::IMAGE_SYM_DTYPE_FUNCTION << COFF::SCT_COMPLEX_TYPE_SHIFT);
- if (Proc.Distance == PROC_DISTANCE_FAR)
- Sym->setIsFarProc();
-
- cast<MCMasmParser>(getParser())
- .setDefaultRetIsFar(Proc.Distance == PROC_DISTANCE_FAR);
- if (Proc.IsFramed) {
+ bool Framed = false;
+ if (getLexer().is(AsmToken::Identifier) &&
+ getTok().getString().equals_insensitive("frame")) {
+ Lex();
+ Framed = true;
getStreamer().emitWinCFIStartProc(Sym, Loc);
}
getStreamer().emitLabel(Sym, Loc);
- CurrentProcedures.push_back(std::move(Proc));
+ CurrentProcedures.push_back(Label);
+ CurrentProceduresFramed.push_back(Framed);
return false;
}
bool COFFMasmParser::parseDirectiveEndProc(StringRef Directive, SMLoc Loc) {
@@ -522,18 +487,15 @@ bool COFFMasmParser::parseDirectiveEndProc(StringRef Directive, SMLoc Loc) {
if (CurrentProcedures.empty())
return Error(Loc, "endp outside of procedure block");
- else if (!CurrentProcedures.back().Name.equals_insensitive(Label))
+ else if (!CurrentProcedures.back().equals_insensitive(Label))
return Error(LabelLoc, "endp does not match current procedure '" +
- CurrentProcedures.back().Name + "'");
+ CurrentProcedures.back() + "'");
- if (CurrentProcedures.back().IsFramed) {
+ if (CurrentProceduresFramed.back()) {
getStreamer().emitWinCFIEndProc(Loc);
}
CurrentProcedures.pop_back();
- cast<MCMasmParser>(getParser())
- .setDefaultRetIsFar(!CurrentProcedures.empty() &&
- CurrentProcedures.back().Distance ==
- PROC_DISTANCE_FAR);
+ CurrentProceduresFramed.pop_back();
return false;
}
diff --git a/llvm/lib/MC/MCParser/MasmParser.cpp b/llvm/lib/MC/MCParser/MasmParser.cpp
index 2e3d3d3890add..51ac19c623732 100644
--- a/llvm/lib/MC/MCParser/MasmParser.cpp
+++ b/llvm/lib/MC/MCParser/MasmParser.cpp
@@ -36,7 +36,6 @@
#include "llvm/MC/MCParser/MCAsmLexer.h"
#include "llvm/MC/MCParser/MCAsmParser.h"
#include "llvm/MC/MCParser/MCAsmParserExtension.h"
-#include "llvm/MC/MCParser/MCMasmParser.h"
#include "llvm/MC/MCParser/MCParsedAsmOperand.h"
#include "llvm/MC/MCParser/MCTargetAsmParser.h"
#include "llvm/MC/MCRegisterInfo.h"
@@ -66,7 +65,6 @@
#include <memory>
#include <optional>
#include <sstream>
-#include <stdbool.h>
#include <string>
#include <tuple>
#include <utility>
@@ -375,7 +373,7 @@ FieldInitializer &FieldInitializer::operator=(FieldInitializer &&Initializer) {
/// The concrete assembly parser instance.
// Note that this is a full MCAsmParser, not an MCAsmParserExtension!
// It's a peer of AsmParser, not of COFFAsmParser, WasmAsmParser, etc.
-class MasmParser : public MCMasmParser {
+class MasmParser : public MCAsmParser {
private:
SourceMgr::DiagHandlerTy SavedDiagHandler;
void *SavedDiagContext;
@@ -450,9 +448,6 @@ class MasmParser : public MCMasmParser {
/// Are we parsing ms-style inline assembly?
bool ParsingMSInlineAsm = false;
- /// Is the current default `ret` instruction far?
- bool DefaultRetIsFar = false;
-
// Current <...> expression depth.
unsigned AngleBracketDepth = 0U;
@@ -478,14 +473,6 @@ class MasmParser : public MCMasmParser {
DirectiveKindMap[Directive] = DirectiveKindMap[Alias];
}
- /// @name MCMasmParser Interface
- /// {
-
- bool getDefaultRetIsFar() const override { return DefaultRetIsFar; }
- void setDefaultRetIsFar(bool IsFar) override { DefaultRetIsFar = IsFar; }
-
- /// }
-
/// @name MCAsmParser Interface
/// {
@@ -517,6 +504,8 @@ class MasmParser : public MCMasmParser {
}
bool isParsingMSInlineAsm() override { return ParsingMSInlineAsm; }
+ bool isParsingMasm() const override { return true; }
+
bool defineMacro(StringRef Name, StringRef Value) override;
bool lookUpField(StringRef Name, AsmFieldInfo &Info) const override;
diff --git a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
index 3c52997d08c7e..8221679f1969c 100644
--- a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -25,7 +25,6 @@
#include "llvm/MC/MCInstrInfo.h"
#include "llvm/MC/MCParser/MCAsmLexer.h"
#include "llvm/MC/MCParser/MCAsmParser.h"
-#include "llvm/MC/MCParser/MCMasmParser.h"
#include "llvm/MC/MCParser/MCParsedAsmOperand.h"
#include "llvm/MC/MCParser/MCTargetAsmParser.h"
#include "llvm/MC/MCRegisterInfo.h"
@@ -33,7 +32,6 @@
#include "llvm/MC/MCStreamer.h"
#include "llvm/MC/MCSubtargetInfo.h"
#include "llvm/MC/MCSymbol.h"
-#include "llvm/MC/MCSymbolCOFF.h"
#include "llvm/MC/TargetRegistry.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Compiler.h"
@@ -1202,10 +1200,6 @@ class X86AsmParser : public MCTargetAsmParser {
void MatchFPUWaitAlias(SMLoc IDLoc, X86Operand &Op, OperandVector &Operands,
MCStreamer &Out, bool MatchingInlineAsm);
- void MatchMASMFarCallToNear(SMLoc IDLoc, X86Operand &Op,
- OperandVector &Operands, MCStreamer &Out,
- bool MatchingInlineAsm);
-
bool ErrorMissingFeature(SMLoc IDLoc, const FeatureBitset &MissingFeatures,
bool MatchingInlineAsm);
@@ -2744,11 +2738,11 @@ bool X86AsmParser::parseIntelOperand(OperandVector &Operands, StringRef Name) {
if ((BaseReg || IndexReg || RegNo || DefaultBaseReg))
Operands.push_back(X86Operand::CreateMem(
getPointerWidth(), RegNo, Disp, BaseReg, IndexReg, Scale, Start, End,
- Size, DefaultBaseReg, /*SymName=*/SM.getSymName(), /*OpDecl=*/nullptr,
+ Size, DefaultBaseReg, /*SymName=*/StringRef(), /*OpDecl=*/nullptr,
/*FrontendSize=*/0, /*UseUpRegs=*/false, MaybeDirectBranchDest));
else
Operands.push_back(X86Operand::CreateMem(
- getPointerWidth(), Disp, Start, End, Size, /*SymName=*/SM.getSymName(),
+ getPointerWidth(), Disp, Start, End, Size, /*SymName=*/StringRef(),
/*OpDecl=*/nullptr, /*FrontendSize=*/0, /*UseUpRegs=*/false,
MaybeDirectBranchDest));
return false;
@@ -3446,14 +3440,6 @@ bool X86AsmParser::parseInstruction(ParseInstructionInfo &Info, StringRef Name,
}
}
- if (Parser.isParsingMasm() && !is64BitMode()) {
- // MASM implicitly converts "ret" to "retf" in far procedures; this is
- // reflected in the default return type in the MCContext.
- if (PatchedName == "ret" &&
- cast<MCMasmParser>(getParser()).getDefaultRetIsFar())
- PatchedName = "retf";
- }
-
// Determine whether this is an instruction prefix.
// FIXME:
// Enhance prefixes integrity robustness. for example, following forms
@@ -4142,11 +4128,6 @@ bool X86AsmParser::matchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
// First, handle aliases that expand to multiple instructions.
MatchFPUWaitAlias(IDLoc, static_cast<X86Operand &>(*Operands[0]), Operands,
Out, MatchingInlineAsm);
- if (getParser().isParsingMasm() && !is64BitMode()) {
- MatchMASMFarCallToNear(IDLoc, static_cast<X86Operand &>(*Operands[0]),
- Operands, Out, MatchingInlineAsm);
- }
-
unsigned Prefixes = getPrefixes(Operands);
MCInst Inst;
@@ -4208,37 +4189,6 @@ void X86AsmParser::MatchFPUWaitAlias(SMLoc IDLoc, X86Operand &Op,
}
}
-void X86AsmParser::MatchMASMFarCallToNear(SMLoc IDLoc, X86Operand &Op,
- OperandVector &Operands,
- MCStreamer &Out,
- bool MatchingInlineAsm) {
- // FIXME: This should be replaced with a real .td file alias mechanism.
- // Also, MatchInstructionImpl should actually *do* the EmitInstruction
- // call.
- if (Op.getToken() != "call")
- return;
- // This is a call instruction...
-
- X86Operand &Operand = static_cast<X86Operand &>(*Operands[1]);
- MCSymbol *Sym = getContext().lookupSymbol(Operand.getSymName());
- if (Sym == nullptr || !Sym->isInSection() || !Sym->isCOFF() ||
- !dyn_cast<MCSymbolCOFF>(Sym)->isFarProc())
- return;
- // Sym is a reference to a far proc in a code section....
-
- if (Out.getCurrentSectionOnly() == &Sym->getSection()) {
- // This is a call to a symbol declared as a far proc, and will be emitted as
- // a near call... so we need to explicitly push the code section register
- // before the call.
- MCInst Inst;
- Inst.setOpcode(X86::PUSH32r);
- Inst.addOperand(MCOperand::createReg(MCRegister(X86::CS)));
- Inst.setLoc(IDLoc);
- if (!MatchingInlineAsm)
- emitInstruction(Inst, Operands, Out);
- }
-}
-
bool X86AsmParser::ErrorMissingFeature(SMLoc IDLoc,
const FeatureBitset &MissingFeatures,
bool MatchingInlineAsm) {
diff --git a/llvm/test/tools/llvm-ml/proc_distance.asm b/llvm/test/tools/llvm-ml/proc_distance.asm
deleted file mode 100644
index 71db903640b42..0000000000000
--- a/llvm/test/tools/llvm-ml/proc_distance.asm
+++ /dev/null
@@ -1,56 +0,0 @@
-; RUN: llvm-ml -m32 -filetype=s %s /Fo - | FileCheck %s
-
-.code
-
-DefaultProc PROC
- ret
-DefaultProc ENDP
-; CHECK: DefaultProc:
-; CHECK: {{^ *}}ret{{ *$}}
-
-t1:
-call DefaultProc
-; CHECK: t1:
-; CHECK-NEXT: call DefaultProc
-
-NearProc PROC NEAR
- ret
-NearProc ENDP
-; CHECK: NearProc:
-; CHECK: {{^ *}}ret{{ *$}}
-
-t2:
-call NearProc
-; CHECK: t2:
-; CHECK-NEXT: call NearProc
-
-FarProcInCode PROC FAR
- ret
-FarProcInCode ENDP
-; CHECK: FarProcInCode:
-; CHECK: {{^ *}}retf{{ *$}}
-
-t3:
-call FarProcInCode
-; CHECK: t3:
-; CHECK-NEXT: push cs
-; CHECK-NEXT: call FarProcInCode
-
-FarCode SEGMENT SHARED NOPAGE NOCACHE INFO READ WRITE EXECUTE DISCARD
-
-FarProcInFarCode PROC FAR
- ret
-FarProcInFarCode ENDP
-; CHECK: FarProcInFarCode:
-; CHECK: {{^ *}}retf{{ *$}}
-
-FarCode ENDS
-
-.code
-
-t4:
-call FarProcInFarCode
-; CHECK: t4:
-; CHECK-NEXT: call FarProcInFarCode
-
-END
More information about the llvm-commits
mailing list