Skip to content

[MC,X86] emitInstruction: remove virtual function calls due to Intel JCC Erratum #96835

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 27, 2024
Merged

[MC,X86] emitInstruction: remove virtual function calls due to Intel JCC Erratum #96835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c4143d4
[𝘀𝗽𝗿] initial version
MaskRay Jun 27, 2024
bb02038
restore support for coff
MaskRay Jun 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions llvm/include/llvm/MC/MCAsmBackend.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,6 @@ class MCAsmBackend {
/// tricky way for optimization.
virtual bool allowEnhancedRelaxation() const { return false; }

/// Give the target a chance to manipulate state related to instruction
/// alignment (e.g. padding for optimization), instruction relaxablility, etc.
/// before and after actually emitting the instruction.
virtual void emitInstructionBegin(MCObjectStreamer &OS, const MCInst &Inst,
const MCSubtargetInfo &STI) {}
virtual void emitInstructionEnd(MCObjectStreamer &OS, const MCInst &Inst) {}

/// lifetime management
virtual void reset() {}

Expand Down
2 changes: 0 additions & 2 deletions llvm/lib/MC/MCObjectStreamer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,9 +330,7 @@ void MCObjectStreamer::emitInstruction(const MCInst &Inst,
"' cannot have instructions");
return;
}
getAssembler().getBackend().emitInstructionBegin(*this, Inst, STI);
emitInstructionImpl(Inst, STI);
getAssembler().getBackend().emitInstructionEnd(*this, Inst);
}

void MCObjectStreamer::emitInstructionImpl(const MCInst &Inst,
Expand Down
41 changes: 38 additions & 3 deletions llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
//===----------------------------------------------------------------------===//

#include "MCTargetDesc/X86BaseInfo.h"
#include "MCTargetDesc/X86FixupKinds.h"
#include "MCTargetDesc/X86EncodingOptimization.h"
#include "MCTargetDesc/X86FixupKinds.h"
#include "llvm/ADT/StringSwitch.h"
#include "llvm/BinaryFormat/ELF.h"
#include "llvm/BinaryFormat/MachO.h"
Expand All @@ -19,6 +19,7 @@
#include "llvm/MC/MCContext.h"
#include "llvm/MC/MCDwarf.h"
#include "llvm/MC/MCELFObjectWriter.h"
#include "llvm/MC/MCELFStreamer.h"
#include "llvm/MC/MCExpr.h"
#include "llvm/MC/MCFixupKindInfo.h"
#include "llvm/MC/MCInst.h"
Expand Down Expand Up @@ -162,8 +163,8 @@ class X86AsmBackend : public MCAsmBackend {
bool allowAutoPadding() const override;
bool allowEnhancedRelaxation() const override;
void emitInstructionBegin(MCObjectStreamer &OS, const MCInst &Inst,
const MCSubtargetInfo &STI) override;
void emitInstructionEnd(MCObjectStreamer &OS, const MCInst &Inst) override;
const MCSubtargetInfo &STI);
void emitInstructionEnd(MCObjectStreamer &OS, const MCInst &Inst);

unsigned getNumFixupKinds() const override {
return X86::NumTargetFixupKinds;
Expand Down Expand Up @@ -1546,3 +1547,37 @@ MCAsmBackend *llvm::createX86_64AsmBackend(const Target &T,
return new ELFX86_X32AsmBackend(T, OSABI, STI);
return new ELFX86_64AsmBackend(T, OSABI, STI);
}

namespace {
class X86ELFStreamer : public MCELFStreamer {
public:
X86ELFStreamer(MCContext &Context, std::unique_ptr<MCAsmBackend> TAB,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about COFF streamer?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's in llvm/lib/Target/X86/MCTargetDesc/X86WinCOFFStreamer.cpp, which doesn't support Intel JCC Erratum.

There is some way to split X86AsmBackend.cpp into X86ELFStreamer.cpp, but that would be a quite large refactoring and otherwise do not yield a noticeable gain.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, Intel JC Erratum supports COFF now

llvm-mc -filetype=obj -triple  x86_64-pc-win32 --x86-align-branch-boundary=32 --x86-align-branch=call+jmp+indirect+ret+jcc ./llvm/test/MC/X86/align-branch-single.s | llvm-objdump -d --no-show-raw-insn -

though I didn't add test for it. Would this patch drop the support for COFF?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added X86_MC::emitInstruction to retain the support for COFF.

std::unique_ptr<MCObjectWriter> OW,
std::unique_ptr<MCCodeEmitter> Emitter)
: MCELFStreamer(Context, std::move(TAB), std::move(OW),
std::move(Emitter)) {}

void emitInstruction(const MCInst &Inst, const MCSubtargetInfo &STI) override;
};
} // end anonymous namespace

void X86_MC::emitInstruction(MCObjectStreamer &S, const MCInst &Inst,
const MCSubtargetInfo &STI) {
auto &Backend = static_cast<X86AsmBackend &>(S.getAssembler().getBackend());
Backend.emitInstructionBegin(S, Inst, STI);
S.MCObjectStreamer::emitInstruction(Inst, STI);
Backend.emitInstructionEnd(S, Inst);
}

void X86ELFStreamer::emitInstruction(const MCInst &Inst,
const MCSubtargetInfo &STI) {
X86_MC::emitInstruction(*this, Inst, STI);
}

MCStreamer *llvm::createX86ELFStreamer(const Triple &T, MCContext &Context,
std::unique_ptr<MCAsmBackend> &&MAB,
std::unique_ptr<MCObjectWriter> &&MOW,
std::unique_ptr<MCCodeEmitter> &&MCE) {
return new X86ELFStreamer(Context, std::move(MAB), std::move(MOW),
std::move(MCE));
}
1 change: 1 addition & 0 deletions llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,7 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeX86TargetMC() {
TargetRegistry::RegisterNullTargetStreamer(*T, createX86NullTargetStreamer);

TargetRegistry::RegisterCOFFStreamer(*T, createX86WinCOFFStreamer);
TargetRegistry::RegisterELFStreamer(*T, createX86ELFStreamer);

// Register the MCInstPrinter.
TargetRegistry::RegisterMCInstPrinter(*T, createX86MCInstPrinter);
Expand Down
9 changes: 9 additions & 0 deletions llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class MCContext;
class MCInst;
class MCInstPrinter;
class MCInstrInfo;
class MCObjectStreamer;
class MCObjectTargetWriter;
class MCObjectWriter;
class MCRegister;
Expand Down Expand Up @@ -89,6 +90,9 @@ bool needsAddressSizeOverride(const MCInst &MI, const MCSubtargetInfo &STI,
/// do not need to go through TargetRegistry.
MCSubtargetInfo *createX86MCSubtargetInfo(const Triple &TT, StringRef CPU,
StringRef FS);

void emitInstruction(MCObjectStreamer &, const MCInst &Inst,
const MCSubtargetInfo &STI);
}

MCCodeEmitter *createX86MCCodeEmitter(const MCInstrInfo &MCII,
Expand Down Expand Up @@ -123,6 +127,11 @@ MCStreamer *createX86WinCOFFStreamer(MCContext &C,
std::unique_ptr<MCCodeEmitter> &&CE,
bool IncrementalLinkerCompatible);

MCStreamer *createX86ELFStreamer(const Triple &T, MCContext &Context,
std::unique_ptr<MCAsmBackend> &&MAB,
std::unique_ptr<MCObjectWriter> &&MOW,
std::unique_ptr<MCCodeEmitter> &&MCE);

/// Construct an X86 Mach-O object writer.
std::unique_ptr<MCObjectTargetWriter>
createX86MachObjectWriter(bool Is64Bit, uint32_t CPUType, uint32_t CPUSubtype);
Expand Down
6 changes: 6 additions & 0 deletions llvm/lib/Target/X86/MCTargetDesc/X86WinCOFFStreamer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,19 @@ class X86WinCOFFStreamer : public MCWinCOFFStreamer {
std::unique_ptr<MCObjectWriter> OW)
: MCWinCOFFStreamer(C, std::move(AB), std::move(CE), std::move(OW)) {}

void emitInstruction(const MCInst &Inst, const MCSubtargetInfo &STI) override;
void emitWinEHHandlerData(SMLoc Loc) override;
void emitWindowsUnwindTables(WinEH::FrameInfo *Frame) override;
void emitWindowsUnwindTables() override;
void emitCVFPOData(const MCSymbol *ProcSym, SMLoc Loc) override;
void finishImpl() override;
};

void X86WinCOFFStreamer::emitInstruction(const MCInst &Inst,
const MCSubtargetInfo &STI) {
X86_MC::emitInstruction(*this, Inst, STI);
}

void X86WinCOFFStreamer::emitWinEHHandlerData(SMLoc Loc) {
MCStreamer::emitWinEHHandlerData(Loc);

Expand Down
Loading