Commit 98438d86 authored by Seth Brenith's avatar Seth Brenith Committed by Commit Bot

[torque] Generate better code when using `&` operator on bitfields

Sometimes CSA code carefully constructs a mask to check several
bitfields at once. Thus far, such a check has been very awkward to write
in Torque. This change adds a way to do so, using the
non-short-circuiting binary `&` operator. So now you can write an
expression that depends on several bitfields from a bitfield struct,
like `x.a == 5 & x.b & !x.c & x.d == 2` (assuming b is a one-bit value),
and it will be reduced to a single mask and equality check. To
demonstrate a usage of this new reduction, this change ports the trivial
macro IsSimpleObjectMap to Torque. I manually verified that the
generated code for the builtin SetDataProperties, which uses that macro,
is unchanged.

Bug: v8:7793
Change-Id: I4a23e0005d738a6699ea0f2a63f9fd67b01e7026
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2183276
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Reviewed-by: 's avatarTobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67948}
parent d48bbd52
......@@ -8401,16 +8401,6 @@ void CodeStubAssembler::Lookup(TNode<Name> unique_name, TNode<Array> array,
}
}
TNode<BoolT> CodeStubAssembler::IsSimpleObjectMap(TNode<Map> map) {
uint32_t mask = Map::Bits1::HasNamedInterceptorBit::kMask |
Map::Bits1::IsAccessCheckNeededBit::kMask;
// !IsSpecialReceiverType && !IsNamedInterceptor && !IsAccessCheckNeeded
return Select<BoolT>(
IsSpecialReceiverInstanceType(LoadMapInstanceType(map)),
[=] { return Int32FalseConstant(); },
[=] { return IsClearWord32(LoadMapBitField(map), mask); });
}
void CodeStubAssembler::TryLookupPropertyInSimpleObject(
TNode<JSObject> object, TNode<Map> map, TNode<Name> unique_name,
Label* if_found_fast, Label* if_found_dict,
......
......@@ -2659,9 +2659,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
TNode<BoolT> IsCustomElementsReceiverInstanceType(
TNode<Int32T> instance_type);
TNode<BoolT> IsSpecialReceiverMap(SloppyTNode<Map> map);
// Returns true if the map corresponds to non-special fast or dictionary
// object.
TNode<BoolT> IsSimpleObjectMap(TNode<Map> map);
TNode<BoolT> IsStringInstanceType(SloppyTNode<Int32T> instance_type);
TNode<BoolT> IsString(SloppyTNode<HeapObject> object);
TNode<BoolT> IsSymbolInstanceType(SloppyTNode<Int32T> instance_type);
......
......@@ -3035,7 +3035,7 @@ void InstructionSelector::VisitUnreachable(Node* node) {
void InstructionSelector::VisitStaticAssert(Node* node) {
Node* asserted = node->InputAt(0);
asserted->Print(2);
asserted->Print(4);
FATAL("Expected turbofan static assert to hold, but got non-true input!\n");
}
......
......@@ -311,35 +311,7 @@ Reduction MachineOperatorReducer::Reduce(Node* node) {
break;
}
case IrOpcode::kWord32Equal: {
Int32BinopMatcher m(node);
if (m.IsFoldable()) { // K == K => K
return ReplaceBool(m.left().Value() == m.right().Value());
}
if (m.left().IsInt32Sub() && m.right().Is(0)) { // x - y == 0 => x == y
Int32BinopMatcher msub(m.left().node());
node->ReplaceInput(0, msub.left().node());
node->ReplaceInput(1, msub.right().node());
return Changed(node);
}
// TODO(turbofan): fold HeapConstant, ExternalReference, pointer compares
if (m.LeftEqualsRight()) return ReplaceBool(true); // x == x => true
if (m.right().HasValue()) {
base::Optional<std::pair<Node*, uint32_t>> replacements;
if (m.left().IsTruncateInt64ToInt32()) {
replacements = ReduceWord32EqualForConstantRhs<Word64Adapter>(
NodeProperties::GetValueInput(m.left().node(), 0),
static_cast<uint32_t>(m.right().Value()));
} else {
replacements = ReduceWord32EqualForConstantRhs<Word32Adapter>(
m.left().node(), static_cast<uint32_t>(m.right().Value()));
}
if (replacements) {
node->ReplaceInput(0, replacements->first);
node->ReplaceInput(1, Uint32Constant(replacements->second));
return Changed(node);
}
}
break;
return ReduceWord32Equal(node);
}
case IrOpcode::kWord64Equal: {
Int64BinopMatcher m(node);
......@@ -1623,9 +1595,117 @@ Reduction MachineOperatorReducer::ReduceWordNAnd(Node* node) {
return NoChange();
}
namespace {
// Represents an operation of the form `(source & mask) == masked_value`.
struct BitfieldCheck {
Node* source;
uint32_t mask;
uint32_t masked_value;
bool truncate_from_64_bit;
static base::Optional<BitfieldCheck> Detect(Node* node) {
// There are two patterns to check for here:
// 1. Single-bit checks: `(val >> shift) & 1`, where:
// - the shift may be omitted, and/or
// - the result may be truncated from 64 to 32
// 2. Equality checks: `(val & mask) == expected`, where:
// - val may be truncated from 64 to 32 before masking (see
// ReduceWord32EqualForConstantRhs)
if (node->opcode() == IrOpcode::kWord32Equal) {
Uint32BinopMatcher eq(node);
if (eq.left().IsWord32And()) {
Uint32BinopMatcher mand(eq.left().node());
if (mand.right().HasValue()) {
BitfieldCheck result{mand.left().node(), mand.right().Value(),
eq.right().Value(), false};
if (mand.left().IsTruncateInt64ToInt32()) {
result.truncate_from_64_bit = true;
result.source =
NodeProperties::GetValueInput(mand.left().node(), 0);
}
return result;
}
}
} else {
if (node->opcode() == IrOpcode::kTruncateInt64ToInt32) {
return TryDetectShiftAndMaskOneBit<Word64Adapter>(
NodeProperties::GetValueInput(node, 0));
} else {
return TryDetectShiftAndMaskOneBit<Word32Adapter>(node);
}
}
return {};
}
base::Optional<BitfieldCheck> TryCombine(const BitfieldCheck& other) {
if (source != other.source ||
truncate_from_64_bit != other.truncate_from_64_bit)
return {};
uint32_t overlapping_bits = mask & other.mask;
// It would be kind of strange to have any overlapping bits, but they can be
// allowed as long as they don't require opposite values in the same
// positions.
if ((masked_value & overlapping_bits) !=
(other.masked_value & overlapping_bits))
return {};
return BitfieldCheck{source, mask | other.mask,
masked_value | other.masked_value,
truncate_from_64_bit};
}
private:
template <typename WordNAdapter>
static base::Optional<BitfieldCheck> TryDetectShiftAndMaskOneBit(Node* node) {
// Look for the pattern `(val >> shift) & 1`. The shift may be omitted.
if (WordNAdapter::IsWordNAnd(NodeMatcher(node))) {
typename WordNAdapter::IntNBinopMatcher mand(node);
if (mand.right().HasValue() && mand.right().Value() == 1) {
if (WordNAdapter::IsWordNShr(mand.left()) ||
WordNAdapter::IsWordNSar(mand.left())) {
typename WordNAdapter::UintNBinopMatcher shift(mand.left().node());
if (shift.right().HasValue() && shift.right().Value() < 32u) {
uint32_t mask = 1 << shift.right().Value();
return BitfieldCheck{shift.left().node(), mask, mask,
WordNAdapter::WORD_SIZE == 64};
}
}
return BitfieldCheck{mand.left().node(), 1, 1,
WordNAdapter::WORD_SIZE == 64};
}
}
return {};
}
};
} // namespace
Reduction MachineOperatorReducer::ReduceWord32And(Node* node) {
DCHECK_EQ(IrOpcode::kWord32And, node->opcode());
return ReduceWordNAnd<Word32Adapter>(node);
Reduction reduction = ReduceWordNAnd<Word32Adapter>(node);
if (reduction.Changed()) {
return reduction;
}
// Attempt to detect multiple bitfield checks from the same bitfield struct
// and fold them into a single check.
Int32BinopMatcher m(node);
if (auto right_bitfield = BitfieldCheck::Detect(m.right().node())) {
if (auto left_bitfield = BitfieldCheck::Detect(m.left().node())) {
if (auto combined_bitfield = left_bitfield->TryCombine(*right_bitfield)) {
Node* source = combined_bitfield->source;
if (combined_bitfield->truncate_from_64_bit) {
source = TruncateInt64ToInt32(source);
}
node->ReplaceInput(0, Word32And(source, combined_bitfield->mask));
node->ReplaceInput(1, Int32Constant(combined_bitfield->masked_value));
NodeProperties::ChangeOp(node, machine()->Word32Equal());
return Changed(node).FollowedBy(ReduceWord32Equal(node));
}
}
}
return NoChange();
}
Reduction MachineOperatorReducer::ReduceWord64And(Node* node) {
......@@ -1756,6 +1836,39 @@ Reduction MachineOperatorReducer::ReduceWord64Xor(Node* node) {
return ReduceWordNXor<Word64Adapter>(node);
}
Reduction MachineOperatorReducer::ReduceWord32Equal(Node* node) {
Int32BinopMatcher m(node);
if (m.IsFoldable()) { // K == K => K
return ReplaceBool(m.left().Value() == m.right().Value());
}
if (m.left().IsInt32Sub() && m.right().Is(0)) { // x - y == 0 => x == y
Int32BinopMatcher msub(m.left().node());
node->ReplaceInput(0, msub.left().node());
node->ReplaceInput(1, msub.right().node());
return Changed(node);
}
// TODO(turbofan): fold HeapConstant, ExternalReference, pointer compares
if (m.LeftEqualsRight()) return ReplaceBool(true); // x == x => true
if (m.right().HasValue()) {
base::Optional<std::pair<Node*, uint32_t>> replacements;
if (m.left().IsTruncateInt64ToInt32()) {
replacements = ReduceWord32EqualForConstantRhs<Word64Adapter>(
NodeProperties::GetValueInput(m.left().node(), 0),
static_cast<uint32_t>(m.right().Value()));
} else {
replacements = ReduceWord32EqualForConstantRhs<Word32Adapter>(
m.left().node(), static_cast<uint32_t>(m.right().Value()));
}
if (replacements) {
node->ReplaceInput(0, replacements->first);
node->ReplaceInput(1, Uint32Constant(replacements->second));
return Changed(node);
}
}
return NoChange();
}
Reduction MachineOperatorReducer::ReduceFloat64InsertLowWord32(Node* node) {
DCHECK_EQ(IrOpcode::kFloat64InsertLowWord32, node->opcode());
Float64Matcher mlhs(node->InputAt(0));
......
......@@ -109,6 +109,7 @@ class V8_EXPORT_PRIVATE MachineOperatorReducer final
Reduction ReduceWord64Or(Node* node);
Reduction ReduceWord32Xor(Node* node);
Reduction ReduceWord64Xor(Node* node);
Reduction ReduceWord32Equal(Node* node);
Reduction ReduceFloat64InsertLowWord32(Node* node);
Reduction ReduceFloat64InsertHighWord32(Node* node);
Reduction ReduceFloat64Compare(Node* node);
......
......@@ -80,3 +80,16 @@ extern class Map extends HeapObject {
macro LoadMapPrototypeInfo(m: Map): PrototypeInfo labels HasNoPrototypeInfo {
return m.PrototypeInfo() otherwise HasNoPrototypeInfo;
}
// Returns true if the map corresponds to non-special fast or dictionary
// object.
@export
macro IsSimpleObjectMap(map: Map): bool {
if (IsSpecialReceiverInstanceType(map.instance_type)) {
return false;
}
const bitField = map.bit_field;
return !bitField.has_named_interceptor & !bitField.is_access_check_needed;
}
extern macro IsSpecialReceiverInstanceType(InstanceType): bool;
......@@ -2,7 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "src/torque/implementation-visitor.h"
#include <algorithm>
#include <iomanip>
#include <string>
#include "src/base/optional.h"
......@@ -10,7 +13,6 @@
#include "src/torque/csa-generator.h"
#include "src/torque/declaration-visitor.h"
#include "src/torque/global-context.h"
#include "src/torque/implementation-visitor.h"
#include "src/torque/parameter-difference.h"
#include "src/torque/server-data.h"
#include "src/torque/type-inference.h"
......@@ -787,7 +789,10 @@ VisitResult ImplementationVisitor::Visit(NumberLiteralExpression* expr) {
}
}
}
return VisitResult{result_type, ToString(expr->number)};
std::stringstream str;
str << std::setprecision(std::numeric_limits<double>::digits10 + 1)
<< expr->number;
return VisitResult{result_type, str.str()};
}
VisitResult ImplementationVisitor::Visit(AssumeTypeImpossibleExpression* expr) {
......
......@@ -756,6 +756,26 @@ TEST(TestBitFieldUintptrOps) {
ft.Call(ft.Val(val2), ft.Val(val3));
}
TEST(TestBitFieldMultipleFlags) {
CcTest::InitializeVM();
Isolate* isolate(CcTest::i_isolate());
i::HandleScope scope(isolate);
const int kNumParams = 3;
CodeAssemblerTester asm_tester(isolate, kNumParams);
TestTorqueAssembler m(asm_tester.state());
{
TNode<BoolT> a =
m.UncheckedCast<BoolT>(m.Unsigned(m.SmiToInt32(m.Parameter(0))));
TNode<Int32T> b = m.SmiToInt32(m.Parameter(1));
TNode<BoolT> c =
m.UncheckedCast<BoolT>(m.Unsigned(m.SmiToInt32(m.Parameter(2))));
m.TestBitFieldMultipleFlags(a, b, c);
m.Return(m.UndefinedConstant());
}
FunctionTester ft(asm_tester.GenerateCode(), kNumParams);
// No need to call it; we just checked StaticAsserts during compilation.
}
TEST(TestTestParentFrameArguments) {
CcTest::InitializeVM();
Isolate* isolate(CcTest::i_isolate());
......
......@@ -1118,6 +1118,42 @@ macro TestBitFieldUintptrOps(
check(val3.e == 1234);
}
bitfield struct TestBitFieldStruct4 extends uint31 {
a: bool: 1 bit;
b: int32: 3 bit;
c: bool: 1 bit;
}
bitfield struct TestBitFieldStruct5 extends uint31 {
b: int32: 19 bit;
a: bool: 1 bit;
c: bool: 1 bit;
}
@export
macro TestBitFieldMultipleFlags(a: bool, b: int32, c: bool) {
const f = TestBitFieldStruct4{a: a, b: b, c: c};
let simpleExpression = f.a & f.b == 3 & !f.c;
let expectedReduction = (Signed(f) & 0x1f) == Convert<int32>(1 | 3 << 1);
StaticAssert(simpleExpression == expectedReduction);
simpleExpression = !f.a & f.b == 4 & f.c;
expectedReduction = (Signed(f) & 0x1f) == Convert<int32>(4 << 1 | 1 << 4);
StaticAssert(simpleExpression == expectedReduction);
simpleExpression = f.b == 0 & f.c;
expectedReduction = (Signed(f) & 0x1e) == Convert<int32>(1 << 4);
StaticAssert(simpleExpression == expectedReduction);
simpleExpression = f.a & f.c;
expectedReduction = (Signed(f) & 0x11) == Convert<int32>(1 | 1 << 4);
StaticAssert(simpleExpression == expectedReduction);
const f2 = TestBitFieldStruct5{b: b, a: a, c: c};
simpleExpression = !f2.a & f2.b == 1234 & f2.c;
expectedReduction = (Signed(f2) & 0x1fffff) == Convert<int32>(1234 | 1 << 20);
StaticAssert(simpleExpression == expectedReduction);
simpleExpression = !f2.a & !f2.c;
expectedReduction = (Signed(f2) & 0x180000) == Convert<int32>(0);
StaticAssert(simpleExpression == expectedReduction);
}
@export
class ExportedSubClass extends ExportedSubClassBase {
c_field: int32;
......
......@@ -765,6 +765,78 @@ TEST_F(MachineOperatorReducerTest, Word32AndWithComparisonAndConstantOne) {
}
}
TEST_F(MachineOperatorReducerTest, Word32AndWithBitFields) {
Node* const p = Parameter(0);
for (int i = 0; i < 2; ++i) {
bool truncate_from_64_bit = i == 1;
auto truncate = [&](Node* const input) {
return truncate_from_64_bit
? graph()->NewNode(machine()->TruncateInt64ToInt32(), input)
: input;
};
// Simulate getting some bitfields from a Torque bitfield struct and
// checking them all, like `x.a == 5 & x.b & !x.c & x.d == 2`. This is
// looking for the pattern: xxxxxxxxxxxxxxxxxxxx10xxx0x1x101. The inputs are
// in an already-reduced state as would be created by
// ReduceWord32EqualForConstantRhs, so the only shift operation remaining is
// the one for selecting a single true bit.
Node* three_bits =
graph()->NewNode(machine()->Word32Equal(), Int32Constant(5),
graph()->NewNode(machine()->Word32And(),
Int32Constant(7), truncate(p)));
Node* single_bit_true =
truncate_from_64_bit
? truncate(graph()->NewNode(machine()->Word64And(),
Int64Constant(1),
graph()->NewNode(machine()->Word64Shr(),
p, Int64Constant(4))))
: graph()->NewNode(machine()->Word32And(), Int32Constant(1),
graph()->NewNode(machine()->Word32Shr(), p,
Int32Constant(4)));
Node* single_bit_false =
graph()->NewNode(machine()->Word32Equal(), Int32Constant(0),
graph()->NewNode(machine()->Word32And(),
Int32Constant(1 << 6), truncate(p)));
Node* two_bits =
graph()->NewNode(machine()->Word32Equal(), Int32Constant(2 << 10),
graph()->NewNode(machine()->Word32And(),
Int32Constant(3 << 10), truncate(p)));
Reduction r1 = Reduce(
graph()->NewNode(machine()->Word32And(), three_bits, single_bit_true));
ASSERT_TRUE(r1.Changed());
EXPECT_THAT(
r1.replacement(),
IsWord32Equal(
IsWord32And(truncate_from_64_bit ? IsTruncateInt64ToInt32(p) : p,
IsInt32Constant(7 | (1 << 4))),
IsInt32Constant(5 | (1 << 4))));
Reduction r2 = Reduce(
graph()->NewNode(machine()->Word32And(), single_bit_false, two_bits));
ASSERT_TRUE(r2.Changed());
EXPECT_THAT(
r2.replacement(),
IsWord32Equal(
IsWord32And(truncate_from_64_bit ? IsTruncateInt64ToInt32(p) : p,
IsInt32Constant((1 << 6) | (3 << 10))),
IsInt32Constant(2 << 10)));
Reduction const r3 = Reduce(graph()->NewNode(
machine()->Word32And(), r1.replacement(), r2.replacement()));
ASSERT_TRUE(r3.Changed());
EXPECT_THAT(
r3.replacement(),
IsWord32Equal(
IsWord32And(truncate_from_64_bit ? IsTruncateInt64ToInt32(p) : p,
IsInt32Constant(7 | (1 << 4) | (1 << 6) | (3 << 10))),
IsInt32Constant(5 | (1 << 4) | (2 << 10))));
}
}
// -----------------------------------------------------------------------------
// Word32Or
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment