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 1 commit
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
36 changes: 33 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,9 @@
//===----------------------------------------------------------------------===//

#include "MCTargetDesc/X86BaseInfo.h"
#include "MCTargetDesc/X86FixupKinds.h"
#include "MCTargetDesc/X86ELFStreamer.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 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,32 @@ 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 X86ELFStreamer::emitInstruction(const MCInst &Inst,
const MCSubtargetInfo &STI) {
auto &Backend = static_cast<X86AsmBackend &>(getAssembler().getBackend());
Backend.emitInstructionBegin(*this, Inst, STI);
MCObjectStreamer::emitInstruction(Inst, STI);
Backend.emitInstructionEnd(*this, Inst);
}

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));
}
22 changes: 22 additions & 0 deletions llvm/lib/Target/X86/MCTargetDesc/X86ELFStreamer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
//===-- X86ELFStreamer.h - ELF Streamer for X86 -----------------*- 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_LIB_TARGET_X86_MCTARGETDESC_X86ELFSTREAMER_H
#define LLVM_LIB_TARGET_X86_MCTARGETDESC_X86ELFSTREAMER_H

#include "llvm/MC/MCELFStreamer.h"

namespace llvm {

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

#endif
2 changes: 2 additions & 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 @@ -14,6 +14,7 @@
#include "TargetInfo/X86TargetInfo.h"
#include "X86ATTInstPrinter.h"
#include "X86BaseInfo.h"
#include "X86ELFStreamer.h"
#include "X86IntelInstPrinter.h"
#include "X86MCAsmInfo.h"
#include "X86TargetStreamer.h"
Expand Down Expand Up @@ -741,6 +742,7 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeX86TargetMC() {
// Register the null streamer.
TargetRegistry::RegisterNullTargetStreamer(*T, createX86NullTargetStreamer);

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

// Register the MCInstPrinter.
Expand Down