Skip to content

Commit

Permalink
Merge pull request #3550 from alyssarosenzweig/ra/validate
Browse files Browse the repository at this point in the history 
Validate that we have no crossblock liveness
  • Loading branch information
@Sonicadvance1
Sonicadvance1 authored Apr 8, 2024
2 parents bb24e14 + 063954c commit e91e1d5
Show file tree
Hide file tree
Showing 5 changed files with 172 additions and 279 deletions.
2 changes: 1 addition & 1 deletion FEXCore/Source/Interface/Core/Core.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ namespace FEXCore::Context {

auto CodeChanged = Thread->OpDispatcher->_ValidateCode(ExistingCodePtr[0], ExistingCodePtr[1], (uintptr_t)ExistingCodePtr - GuestRIP, DecodedInfo->InstSize);

auto InvalidateCodeCond = Thread->OpDispatcher->_CondJump(CodeChanged);
auto InvalidateCodeCond = Thread->OpDispatcher->CondJump(CodeChanged);

auto CurrentBlock = Thread->OpDispatcher->GetCurrentBlock();
auto CodeWasChangedBlock = Thread->OpDispatcher->CreateNewCodeBlockAtEnd();
Expand Down
22 changes: 12 additions & 10 deletions FEXCore/Source/Interface/Core/OpcodeDispatcher/Vector.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3008,10 +3008,10 @@ OrderedNode *OpDispatchBuilder::XSaveBase(X86Tables::DecodedOp Op) {
void OpDispatchBuilder::XSaveOpImpl(OpcodeArgs) {
// NOTE: Mask should be EAX and EDX concatenated, but we only need to test
// for features that are in the lower 32 bits, so EAX only is sufficient.
OrderedNode *Mask = LoadGPRRegister(X86State::REG_RAX);
const auto OpSize = IR::SizeToOpSize(CTX->GetGPRSize());

const auto StoreIfFlagSet = [&](uint32_t BitIndex, auto fn, uint32_t FieldSize = 1){
const auto StoreIfFlagSet = [this, OpSize](uint32_t BitIndex, auto fn, uint32_t FieldSize = 1){
OrderedNode *Mask = LoadGPRRegister(X86State::REG_RAX);
OrderedNode *BitFlag = _Bfe(OpSize, FieldSize, BitIndex, Mask);
auto CondJump_ = CondJump(BitFlag, {COND_NEQ});

Expand Down Expand Up @@ -3055,6 +3055,7 @@ void OpDispatchBuilder::XSaveOpImpl(OpcodeArgs) {
OrderedNode *HeaderOffset = _Add(OpSize, Base, _Constant(512));

// NOTE: We currently only support the first 3 bits (x87, SSE, and AVX)
OrderedNode *Mask = LoadGPRRegister(X86State::REG_RAX);
OrderedNode *RequestedFeatures = _Bfe(OpSize, 3, 0, Mask);

// XSTATE_BV section of the header is 8 bytes in size, but we only really
Expand Down Expand Up @@ -3209,17 +3210,18 @@ void OpDispatchBuilder::FXRStoreOp(OpcodeArgs) {
void OpDispatchBuilder::XRstorOpImpl(OpcodeArgs) {
const auto OpSize = IR::SizeToOpSize(CTX->GetGPRSize());

// Set up base address for the XSAVE region to restore from, and also read the
// XSTATE_BV bit flags out of the XSTATE header.
//
// Note: we rematerialize Base in each block to avoid crossblock liveness.
OrderedNode *Base = XSaveBase(Op);
OrderedNode *Mask = _LoadMem(GPRClass, 8, _Add(OpSize, Base, _Constant(512)), 8);

// If a bit in our XSTATE_BV is set, then we restore from that region of the XSAVE area,
// otherwise, if not set, then we need to set the relevant data the bit corresponds to
// to it's defined initial configuration.
const auto RestoreIfFlagSetOrDefault = [&](uint32_t BitIndex, auto restore_fn, auto default_fn, uint32_t FieldSize = 1){
const auto RestoreIfFlagSetOrDefault = [this, Op, OpSize](uint32_t BitIndex, auto restore_fn, auto default_fn, uint32_t FieldSize = 1){
// Set up base address for the XSAVE region to restore from, and also read
// the XSTATE_BV bit flags out of the XSTATE header.
//
// Note: we rematerialize Base/Mask in each block to avoid crossblock
// liveness.
OrderedNode *Base = XSaveBase(Op);
OrderedNode *Mask = _LoadMem(GPRClass, 8, _Add(OpSize, Base, _Constant(512)), 8);

OrderedNode *BitFlag = _Bfe(OpSize, FieldSize, BitIndex, Mask);
auto CondJump_ = CondJump(BitFlag, {COND_NEQ});

Expand Down
169 changes: 29 additions & 140 deletions FEXCore/Source/Interface/IR/Passes/ValueDominanceValidation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,6 @@ desc: Sanity Checking
#include <stdint.h>
#include <utility>

namespace {
struct BlockInfo {
fextl::vector<FEXCore::IR::OrderedNode *> Predecessors;
fextl::vector<FEXCore::IR::OrderedNode *> Successors;
};
}

namespace FEXCore::IR::Validation {
class ValueDominanceValidation final : public FEXCore::IR::Pass {
public:
Expand All @@ -43,50 +36,6 @@ bool ValueDominanceValidation::Run(IREmitter *IREmit) {
auto CurrentIR = IREmit->ViewIR();

fextl::ostringstream Errors;
fextl::unordered_map<IR::NodeID, BlockInfo> OffsetToBlockMap;

for (auto [BlockNode, BlockHeader] : CurrentIR.GetBlocks()) {

BlockInfo *CurrentBlock = &OffsetToBlockMap.try_emplace(CurrentIR.GetID(BlockNode)).first->second;

for (auto [CodeNode, IROp] : CurrentIR.GetCode(BlockNode)) {
switch (IROp->Op) {
case IR::OP_CONDJUMP: {
auto Op = IROp->CW<IR::IROp_CondJump>();

OrderedNode *TrueTargetNode = CurrentIR.GetNode(Op->TrueBlock);
OrderedNode *FalseTargetNode = CurrentIR.GetNode(Op->FalseBlock);

CurrentBlock->Successors.emplace_back(TrueTargetNode);
CurrentBlock->Successors.emplace_back(FalseTargetNode);

{
auto Block = &OffsetToBlockMap.try_emplace(Op->TrueBlock.ID()).first->second;
Block->Predecessors.emplace_back(BlockNode);
}

{
auto Block = &OffsetToBlockMap.try_emplace(Op->FalseBlock.ID()).first->second;
Block->Predecessors.emplace_back(BlockNode);
}

break;
}
case IR::OP_JUMP: {
auto Op = IROp->CW<IR::IROp_Jump>();
OrderedNode *TargetNode = CurrentIR.GetNode(Op->Header.Args[0]);
CurrentBlock->Successors.emplace_back(TargetNode);

{
auto Block = OffsetToBlockMap.try_emplace(Op->Header.Args[0].ID()).first;
Block->second.Predecessors.emplace_back(BlockNode);
}
break;
}
default: break;
}
}
}

for (auto [BlockNode, BlockHeader] : CurrentIR.GetBlocks()) {
auto BlockIROp = BlockHeader->CW<FEXCore::IR::IROp_CodeBlock>();
Expand All @@ -97,99 +46,38 @@ bool ValueDominanceValidation::Run(IREmitter *IREmit) {
const uint8_t NumArgs = IR::GetRAArgs(IROp->Op);
for (uint32_t i = 0; i < NumArgs; ++i) {
if (IROp->Args[i].IsInvalid()) continue;
if (CurrentIR.GetOp<IROp_Header>(IROp->Args[i])->Op == OP_IRHEADER) continue;

// We do not validate the location of inline constants because it's
// irrelevant, they're ignored by RA and always inlined to where they
// need to be. This lets us pool inline constants globally.
IROps Op = CurrentIR.GetOp<IROp_Header>(IROp->Args[i])->Op;
if (Op == OP_IRHEADER || Op == OP_INLINECONSTANT) continue;

OrderedNodeWrapper Arg = IROp->Args[i];

// We must ensure domininance of all SSA arguments
if (Arg.ID() >= BlockIROp->Begin.ID() &&
Arg.ID() < BlockIROp->Last.ID()) {
// If the SSA argument is defined INSIDE this block
// then it must only be declared prior to this instruction
// Eg: Valid
// CodeBlock_1:
// %_1 = Load
// %_2 = Load
// %_3 = <Op> %_1, %_2
//
// Eg: Invalid
// CodeBlock_1:
// %_1 = Load
// %_2 = <Op> %_1, %_3
// %_3 = Load
if (Arg.ID() > CodeID) {
HadError |= true;
Errors << "Inst %" << CodeID << ": Arg[" << i << "] %" << Arg.ID() << " definition does not dominate this use!" << std::endl;
}
}
else if (Arg.ID() < BlockIROp->Begin.ID()) {
// If the SSA argument is defined BEFORE this block
// then THIS block needs to be dominated by the flow of blocks up until this point

// Eg: Valid
// CodeBlock_1:
// %_1 = Load
// %_2 = Load
// Jump %CodeBlock_2
//
// CodeBlock_2:
// %_3 = <Op> %_1, %_2
//
// Eg: Invalid
// CodeBlock_1:
// %_1 = Load
// %_2 = Load
// Jump %CodeBlock_3
//
// CodeBlock_2:
// %_3 = <Op> %_1, %_2
//
// CodeBlock_3:
// ...

// We need to walk the predecessors to see if the value comes from there
fextl::set<IR::OrderedNode *> Predecessors { BlockNode };
// Recursively gather all predecessors of BlockNode
for (auto NodeIt = Predecessors.begin(); NodeIt != Predecessors.end();) {
auto PredBlock = &OffsetToBlockMap.try_emplace(CurrentIR.GetID(*NodeIt)).first->second;
++NodeIt;

for (auto *Pred : PredBlock->Predecessors) {
if (Predecessors.insert(Pred).second) {
// New blocks added, so repeat from the beginning to pull in their predecessors
NodeIt = Predecessors.begin();
}
}
}

bool FoundPredDefine = false;

for (auto* Pred : Predecessors) {
auto PredIROp = CurrentIR.GetOp<FEXCore::IR::IROp_CodeBlock>(Pred);

if (Arg.ID() >= PredIROp->Begin.ID() &&
Arg.ID() < PredIROp->Last.ID()) {
FoundPredDefine = true;
break;
}
Errors << "\tChecking Pred %" << CurrentIR.GetID(Pred) << std::endl;
}

if (!FoundPredDefine) {
HadError |= true;
Errors << "Inst %" << CodeID << ": Arg[" << i << "] %" << Arg.ID() << " definition does not dominate this use! But was defined before this block!" << std::endl;
}
// If the SSA argument is not defined INSIDE the block, we have
// cross-block liveness, which we forbid in the IR to simplify RA.
if (!(Arg.ID() >= BlockIROp->Begin.ID() &&
Arg.ID() < BlockIROp->Last.ID())) {
HadError |= true;
Errors << "Inst %" << CodeID << ": Arg[" << i << "] %" << Arg.ID() << " definition not local!" << std::endl;
continue;
}
else if (Arg.ID() > BlockIROp->Last.ID()) {
// If this SSA argument is defined AFTER this block then it is just completely broken
// Eg: Invalid
// CodeBlock_1:
// %_1 = Load
// %_2 = <Op> %_1, %_3
// Jump %CodeBlock_2
//
// CodeBlock_2:
// %_3 = Load

// The SSA argument is defined INSIDE this block.
// It must only be declared prior to this instruction
// Eg: Valid
// CodeBlock_1:
// %_1 = Load
// %_2 = Load
// %_3 = <Op> %_1, %_2
//
// Eg: Invalid
// CodeBlock_1:
// %_1 = Load
// %_2 = <Op> %_1, %_3
// %_3 = Load
if (Arg.ID() > CodeID) {
HadError |= true;
Errors << "Inst %" << CodeID << ": Arg[" << i << "] %" << Arg.ID() << " definition does not dominate this use!" << std::endl;
}
Expand All @@ -202,6 +90,7 @@ bool ValueDominanceValidation::Run(IREmitter *IREmit) {
FEXCore::IR::Dump(&Out, &CurrentIR, nullptr);
Out << "Errors:" << std::endl << Errors.str() << std::endl;
LogMan::Msg::EFmt("{}", Out.str());
LOGMAN_MSG_A_FMT("Encountered IR validation Error");
}

return false;
Expand Down
Loading

0 comments on commit e91e1d5

Please sign in to comment.