X87: [x64/ia32] Deal with the non-transitivity of InstructionSelector::CanCover() when folding loads into branches.
port 0d22e7e46a
(r36482)
original commit message:
Sequences like:
1: Load[kRepWord32|kTypeInt32](<address>, ...)
2: Word32And(1, <constant>)
3: Word32Equal(2, <another constant>)
4: Store[(kRepWord32 : NoWriteBarrier)](<address>, <value>)
5: Branch[None](3, ...) -> B1, B2
where #1 and #4 refer to the same memory location, are problematic because in VisitBranch we assume that 'InstructionSelector::CanCover()' is transitive.
What happens is that CanCover(5, 3) is true (3 is a pure op), and so are CanCover(3, 2), CanCover(2, 1), but the effect level of 5 and 3 never gets checked because 3 is a pure op. Upon VisitBranch, we
mov [address], <value>
test [address], <another constant>
With this patch, it becomes:
mov reg, [address]
mov [address], <value>
test reg, <another constant>
BUG=
Review-Url: https://codereview.chromium.org/2006223004
Cr-Commit-Position: refs/heads/master@{#36501}
This commit is contained in:
parent
eb488c1f8c
commit
4d9149e1be
@ -27,11 +27,15 @@ class X87OperandGenerator final : public OperandGenerator {
|
|||||||
return DefineAsRegister(node);
|
return DefineAsRegister(node);
|
||||||
}
|
}
|
||||||
|
|
||||||
bool CanBeMemoryOperand(InstructionCode opcode, Node* node, Node* input) {
|
bool CanBeMemoryOperand(InstructionCode opcode, Node* node, Node* input,
|
||||||
|
int effect_level) {
|
||||||
if (input->opcode() != IrOpcode::kLoad ||
|
if (input->opcode() != IrOpcode::kLoad ||
|
||||||
!selector()->CanCover(node, input)) {
|
!selector()->CanCover(node, input)) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
if (effect_level != selector()->GetEffectLevel(input)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
MachineRepresentation rep =
|
MachineRepresentation rep =
|
||||||
LoadRepresentationOf(input->op()).representation();
|
LoadRepresentationOf(input->op()).representation();
|
||||||
switch (opcode) {
|
switch (opcode) {
|
||||||
@ -1268,18 +1272,24 @@ void VisitWordCompare(InstructionSelector* selector, Node* node,
|
|||||||
|
|
||||||
InstructionCode narrowed_opcode = TryNarrowOpcodeSize(opcode, left, right);
|
InstructionCode narrowed_opcode = TryNarrowOpcodeSize(opcode, left, right);
|
||||||
|
|
||||||
|
int effect_level = selector->GetEffectLevel(node);
|
||||||
|
if (cont->IsBranch()) {
|
||||||
|
effect_level = selector->GetEffectLevel(
|
||||||
|
cont->true_block()->PredecessorAt(0)->control_input());
|
||||||
|
}
|
||||||
|
|
||||||
// If one of the two inputs is an immediate, make sure it's on the right, or
|
// If one of the two inputs is an immediate, make sure it's on the right, or
|
||||||
// if one of the two inputs is a memory operand, make sure it's on the left.
|
// if one of the two inputs is a memory operand, make sure it's on the left.
|
||||||
if ((!g.CanBeImmediate(right) && g.CanBeImmediate(left)) ||
|
if ((!g.CanBeImmediate(right) && g.CanBeImmediate(left)) ||
|
||||||
(g.CanBeMemoryOperand(narrowed_opcode, node, right) &&
|
(g.CanBeMemoryOperand(narrowed_opcode, node, right, effect_level) &&
|
||||||
!g.CanBeMemoryOperand(narrowed_opcode, node, left))) {
|
!g.CanBeMemoryOperand(narrowed_opcode, node, left, effect_level))) {
|
||||||
if (!node->op()->HasProperty(Operator::kCommutative)) cont->Commute();
|
if (!node->op()->HasProperty(Operator::kCommutative)) cont->Commute();
|
||||||
std::swap(left, right);
|
std::swap(left, right);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Match immediates on right side of comparison.
|
// Match immediates on right side of comparison.
|
||||||
if (g.CanBeImmediate(right)) {
|
if (g.CanBeImmediate(right)) {
|
||||||
if (g.CanBeMemoryOperand(opcode, node, left)) {
|
if (g.CanBeMemoryOperand(opcode, node, left, effect_level)) {
|
||||||
// TODO(epertoso): we should use `narrowed_opcode' here once we match
|
// TODO(epertoso): we should use `narrowed_opcode' here once we match
|
||||||
// immediates too.
|
// immediates too.
|
||||||
return VisitCompareWithMemoryOperand(selector, opcode, left,
|
return VisitCompareWithMemoryOperand(selector, opcode, left,
|
||||||
@ -1290,7 +1300,7 @@ void VisitWordCompare(InstructionSelector* selector, Node* node,
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Match memory operands on left side of comparison.
|
// Match memory operands on left side of comparison.
|
||||||
if (g.CanBeMemoryOperand(narrowed_opcode, node, left)) {
|
if (g.CanBeMemoryOperand(narrowed_opcode, node, left, effect_level)) {
|
||||||
bool needs_byte_register =
|
bool needs_byte_register =
|
||||||
narrowed_opcode == kX87Test8 || narrowed_opcode == kX87Cmp8;
|
narrowed_opcode == kX87Test8 || narrowed_opcode == kX87Cmp8;
|
||||||
return VisitCompareWithMemoryOperand(
|
return VisitCompareWithMemoryOperand(
|
||||||
|
Loading…
Reference in New Issue
Block a user