Skip to content

Commit

Permalink
Frontend: short-circuit code generation on invalid instructions with …
Browse files Browse the repository at this point in the history
…multiblock

A source of overhead with multiblock is hitting instructions through a
conditional branch that can never be executed. Usually AVX512
instructions in glibc. This causes us to emit partial blocks for a ton
of targets that will never get executed.

Instead, when we have multiblock enabled, if a block hits an instruction
encoding we don't support, then remove all the decoded instructions from
the block and early terminate it if it isn't the entry block. This
resolves the issue of emitting a bunch of IR and code for blocks never
executed.

If the block of code has an invalid instruction in the entry block for
decoding then it'll still emit code up to the invalid instruction and
raise a SIGILL. This has the potential for generating some additional
blocks of code if a game is abusing SIGILL, but since that's unlikely
it's a good trade-off.

Also removes a few log instructions that don't really provide anything
anymore and just show up as confusing messages when multiblock is
enabled.
  • Loading branch information
Sonicadvance1 committed Aug 24, 2024
1 parent 2829ad5 commit c82a683
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 8 deletions.
5 changes: 5 additions & 0 deletions FEXCore/Source/Interface/Core/Core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,11 @@ ContextImpl::GenerateIR(FEXCore::Core::InternalThreadState* Thread, uint64_t Gue

uint64_t InstsInBlock = Block.NumInstructions;

if (InstsInBlock == 0) {
// Special case for an empty instruction block.
Thread->OpDispatcher->ExitFunction(Thread->OpDispatcher->_EntrypointOffset(IR::SizeToOpSize(GPRSize), Block.Entry - GuestRIP));
}

for (size_t i = 0; i < InstsInBlock; ++i) {
const FEXCore::X86Tables::X86InstInfo* TableInfo {nullptr};
const FEXCore::X86Tables::DecodedInst* DecodedInfo {nullptr};
Expand Down
36 changes: 28 additions & 8 deletions FEXCore/Source/Interface/Core/Frontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,12 +274,10 @@ bool Decoder::NormalOp(const FEXCore::X86Tables::X86InstInfo* Info, uint16_t Op,
DecodeInst->TableInfo = Info;

if (Info->Type == FEXCore::X86Tables::TYPE_UNKNOWN) {
LogMan::Msg::DFmt("Unknown instruction: {} 0x{:04x} 0x{:x}", Info->Name ?: "UND", Op, DecodeInst->PC);
return false;
}

if (Info->Type == FEXCore::X86Tables::TYPE_INVALID) {
LogMan::Msg::DFmt("Invalid or Unknown instruction: {} 0x{:04x} 0x{:x}", Info->Name ?: "UND", Op, DecodeInst->PC);
return false;
}

Expand Down Expand Up @@ -557,12 +555,10 @@ bool Decoder::NormalOpHeader(const FEXCore::X86Tables::X86InstInfo* Info, uint16
DecodeInst->TableInfo = Info;

if (Info->Type == FEXCore::X86Tables::TYPE_UNKNOWN) {
LogMan::Msg::DFmt("Unknown instruction: {} 0x{:04x} 0x{:x}", Info->Name ?: "UND", Op, DecodeInst->PC);
return false;
}

if (Info->Type == FEXCore::X86Tables::TYPE_INVALID) {
LogMan::Msg::DFmt("Invalid or Unknown instruction: {} 0x{:04x} 0x{:x}", Info->Name ?: "UND", Op, DecodeInst->PC);
return false;
}

Expand Down Expand Up @@ -977,12 +973,12 @@ void Decoder::BranchTargetInMultiblockRange() {
// If we are conditional then a target can be the instruction past the conditional instruction
uint64_t FallthroughRIP = DecodeInst->PC + DecodeInst->InstSize;
if (!HasBlocks.contains(FallthroughRIP)) {
BlocksToDecode.insert(FallthroughRIP);
CurrentBlockTargets.insert(FallthroughRIP);
}
}

if (!HasBlocks.contains(TargetRIP)) {
BlocksToDecode.insert(TargetRIP);
CurrentBlockTargets.insert(TargetRIP);
}
} else {
if (ExternalBranches) {
Expand Down Expand Up @@ -1079,6 +1075,8 @@ void Decoder::DecodeInstructionsAtEntry(const uint8_t* _InstStream, uint64_t PC,
MaxInst = CTX->Config.MaxInstPerBlock;
}

bool EntryBlock {true};

while (!BlocksToDecode.empty()) {
auto BlockDecodeIt = BlocksToDecode.begin();
uint64_t RIPToDecode = *BlockDecodeIt;
Expand Down Expand Up @@ -1115,22 +1113,39 @@ void Decoder::DecodeInstructionsAtEntry(const uint8_t* _InstStream, uint64_t PC,
bool ErrorDuringDecoding = !DecodeInstruction(RIPToDecode + PCOffset);

if (ErrorDuringDecoding) [[unlikely]] {
LogMan::Msg::DFmt("Couldn't Decode something at 0x{:x}, Started at 0x{:x}", RIPToDecode + PCOffset, PC);
// Put an invalid instruction in the stream so the core can raise SIGILL if hit
CurrentBlockDecoding.HasInvalidInstruction = true;
// Error while decoding instruction. We don't know the table or instruction size
DecodeInst->TableInfo = nullptr;
DecodeInst->InstSize = 0;
}

if (!ErrorDuringDecoding) {
// If there wasn't an error during decoding but we have no dispatcher for the instruction then claim invalid instruction.
auto TableInfo = DecodedBuffer[BlockStartOffset + BlockNumberOfInstructions].TableInfo;
if (!TableInfo || !TableInfo->OpcodeDispatcher) {
CurrentBlockDecoding.HasInvalidInstruction = true;
}
}

DecodedMinAddress = std::min(DecodedMinAddress, RIPToDecode + PCOffset);
DecodedMaxAddress = std::max(DecodedMaxAddress, RIPToDecode + PCOffset + DecodeInst->InstSize);
++TotalInstructions;
++BlockNumberOfInstructions;
++DecodedSize;

// Can not continue this block at all on invalid instruction
if (CurrentBlockDecoding.HasInvalidInstruction) {
if (CurrentBlockDecoding.HasInvalidInstruction) [[unlikely]] {
if (!EntryBlock) {
// In multiblock configurations, we can early terminate any non-entrypoint blocks with the expectation that this won't get hit.
// Improves compile-times.
// Just need to undo additions that this block decoding has caused.
TotalInstructions -= CurrentBlockDecoding.NumInstructions;
DecodedSize = BlockStartOffset;
BlockNumberOfInstructions = 0;
InstStream -= PCOffset;
CurrentBlockTargets.clear();
}
break;
}

Expand Down Expand Up @@ -1160,13 +1175,18 @@ void Decoder::DecodeInstructionsAtEntry(const uint8_t* _InstStream, uint64_t PC,
InstStream += DecodeInst->InstSize;
}

BlocksToDecode.merge(CurrentBlockTargets);
CurrentBlockTargets.clear();

BlocksToDecode.erase(BlockDecodeIt);
HasBlocks.emplace(RIPToDecode);

// Copy over only the number of instructions we decoded
CurrentBlockDecoding.NumInstructions = BlockNumberOfInstructions;
CurrentBlockDecoding.DecodedInstructions = &DecodedBuffer[BlockStartOffset];
BlockInfo.TotalInstructionCount += BlockNumberOfInstructions;

EntryBlock = false;
}

for (auto CodePage : CodePages) {
Expand Down
1 change: 1 addition & 0 deletions FEXCore/Source/Interface/Core/Frontend.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ class Decoder final {
uint64_t SectionMaxAddress {~0ULL};

DecodedBlockInformation BlockInfo;
fextl::set<uint64_t> CurrentBlockTargets;
fextl::set<uint64_t> BlocksToDecode;
fextl::set<uint64_t> HasBlocks;
fextl::set<uint64_t>* ExternalBranches {nullptr};
Expand Down

0 comments on commit c82a683

Please sign in to comment.