Skip to content

Commit

Permalink
ConstProp: fix 32-bit masking behaviour
Browse files Browse the repository at this point in the history
if we want to replace a node with one of its sources, we need to zero extend if
the source is 64-bit and the destination is 32-bit.

Signed-off-by: Alyssa Rosenzweig <[email protected]>
  • Loading branch information
alyssarosenzweig committed Dec 13, 2024
1 parent 51f505a commit 29405f2
Showing 1 changed file with 33 additions and 7 deletions.
40 changes: 33 additions & 7 deletions FEXCore/Source/Interface/IR/Passes/ConstProp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,35 @@ void ConstProp::HandleConstantPools(IREmitter* IREmit, const IRListView& Current
}
}

// Helper to replace the destination of an instruction with one of its sources,
// to implement algebraic identities. This is surprisingly tricky due to
// implicit masking in our IR.
//
// FEX's IR uses sized opcodes, matching arm64 semantics. 64-bit opcodes do not
// mask, whereas smaller opcodes mask/zero-extend from 32-bits. Therefore, if
// the instruction is 32-bit, we need to mask the source for a sound
// replacement, in case there was garbage in the upper bits.
//
// However, if that source is in turn written by a 32-bit instruction, it is
// guaranteed to have already been masked, so we know there's no garbage and we
// can avoid the zero-extension. This is the case 99% of the time, but the
// masking here is correctness-bearing nevertheless (and new versions of Denuvo
// break if you get this wrong!)
static inline void ReplaceWithSource(IREmitter* IREmit, const IRListView& CurrentIR, Ref CodeNode, IROp_Header* IROp, unsigned Idx) {
Ref Arg = CurrentIR.GetNode(IROp->Args[Idx]);

if (IROp->Size < OpSize::i64Bit) {
LOGMAN_THROW_A_FMT(IROp->Size == OpSize::i32Bit, "other sizes not here");

auto Header = IREmit->GetOpHeader(IROp->Args[Idx]);
if (Header->Size > OpSize::i32Bit) {
Arg = IREmit->_Bfe(OpSize::i32Bit, 32, 0, Arg);
}
}

IREmit->ReplaceAllUsesWith(CodeNode, Arg);
}

// constprop + some more per instruction logic
void ConstProp::ConstantPropagation(IREmitter* IREmit, const IRListView& CurrentIR, Ref CodeNode, IROp_Header* IROp) {
switch (IROp->Op) {
Expand Down Expand Up @@ -280,7 +309,7 @@ void ConstProp::ConstantPropagation(IREmitter* IREmit, const IRListView& Current
Replaced = true;
} else if (IROp->Args[0].ID() == IROp->Args[1].ID() || (Constant2 & getMask(IROp)) == getMask(IROp)) {
// AND with same value results in original value
IREmit->ReplaceAllUsesWith(CodeNode, CurrentIR.GetNode(IROp->Args[0]));
ReplaceWithSource(IREmit, CurrentIR, CodeNode, IROp, 0);
Replaced = true;
}

Expand Down Expand Up @@ -313,8 +342,7 @@ void ConstProp::ConstantPropagation(IREmitter* IREmit, const IRListView& Current
}

IREmit->SetWriteCursor(CodeNode);
Ref Arg = CurrentIR.GetNode(IROp->Args[1 - i]);
IREmit->ReplaceAllUsesWith(CodeNode, Arg);
ReplaceWithSource(IREmit, CurrentIR, CodeNode, IROp, 1 - i);
Replaced = true;
break;
}
Expand Down Expand Up @@ -356,8 +384,7 @@ void ConstProp::ConstantPropagation(IREmitter* IREmit, const IRListView& Current
IREmit->ReplaceWithConstant(CodeNode, NewConstant);
} else if (IREmit->IsValueConstant(IROp->Args[1], &Constant2) && Constant2 == 0) {
IREmit->SetWriteCursor(CodeNode);
Ref Arg = CurrentIR.GetNode(IROp->Args[0]);
IREmit->ReplaceAllUsesWith(CodeNode, Arg);
ReplaceWithSource(IREmit, CurrentIR, CodeNode, IROp, 0);
} else {
Inline(IREmit, CurrentIR, CodeNode, IROp, 1);
}
Expand All @@ -368,8 +395,7 @@ void ConstProp::ConstantPropagation(IREmitter* IREmit, const IRListView& Current

if (IREmit->IsValueConstant(IROp->Args[1], &Constant2) && Constant2 == 0) {
IREmit->SetWriteCursor(CodeNode);
Ref Arg = CurrentIR.GetNode(IROp->Args[0]);
IREmit->ReplaceAllUsesWith(CodeNode, Arg);
ReplaceWithSource(IREmit, CurrentIR, CodeNode, IROp, 0);
} else {
Inline(IREmit, CurrentIR, CodeNode, IROp, 1);
}
Expand Down

0 comments on commit 29405f2

Please sign in to comment.