Revert "[vm, compiler] Support unboxed parameters for integer intrinsics."

This reverts commit 1d369b5000.

Reason for revert: Failures on Golem

Original change's description:
> [vm, compiler] Support unboxed parameters for integer intrinsics.
>
> TEST=ci
> Change-Id: I7f29471043c049ef1acf7cd4bcb0890db6d33aa4
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/192728
> Commit-Queue: Ryan Macnak <rmacnak@google.com>
> Reviewed-by: Alexander Markov <alexmarkov@google.com>
> Reviewed-by: Martin Kustermann <kustermann@google.com>

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: I09e54cd894c0f73bf3af215729567503c156ec3e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/195165
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
This commit is contained in:
Ryan Macnak 2021-04-13 17:05:47 +00:00 committed by commit-bot@chromium.org
parent 0a2bd758fc
commit 55645786bc
8 changed files with 84 additions and 106 deletions

View file

@ -13,6 +13,7 @@ const kNonNullableResultType = "vm:non-nullable-result-type";
const kResultTypeUsesPassedTypeArguments =
"result-type-uses-passed-type-arguments";
const kRecognizedPragmaName = "vm:recognized";
const kDisableUnboxedParametetersPragmaName = "vm:disable-unboxed-parameters";
abstract class ParsedPragma {}
@ -46,6 +47,10 @@ class ParsedRecognized extends ParsedPragma {
ParsedRecognized(this.type);
}
class ParsedDisableUnboxedParameters extends ParsedPragma {
ParsedDisableUnboxedParameters();
}
abstract class PragmaAnnotationParser {
/// May return 'null' if the annotation does not represent a recognized
/// @pragma.
@ -142,6 +147,8 @@ class ConstantPragmaAnnotationParser extends PragmaAnnotationParser {
"pragma: $options";
}
return new ParsedRecognized(type);
case kDisableUnboxedParametetersPragmaName:
return new ParsedDisableUnboxedParameters();
default:
return null;
}

View file

@ -233,6 +233,19 @@ class NativeCodeOracle {
(expectedTypes == null || expectedTypes.contains(type));
}
bool hasDisableUnboxedParameters(Member member) {
for (var annotation in member.annotations) {
ParsedPragma pragma = _matcher.parsePragma(annotation);
if (pragma is ParsedDisableUnboxedParameters) {
if (member.enclosingLibrary.importUri.scheme != "dart") {
throw "ERROR: Cannot use @pragma(vm:disable-unboxed-parameters) outside core libraries.";
}
return true;
}
}
return false;
}
/// Simulate the execution of a native method by adding its entry points
/// using [entryPointsListener]. Returns result type of the native method.
TypeExpr handleNativeProcedure(

View file

@ -203,7 +203,8 @@ class UnboxingInfoManager {
_nativeCodeOracle.isRecognized(member, const [
PragmaRecognizedType.AsmIntrinsic,
PragmaRecognizedType.Other
]);
]) ||
_nativeCodeOracle.hasDisableUnboxedParameters(member);
}
bool _isNative(Member member) => getExternalName(member) != null;

View file

@ -5664,9 +5664,7 @@ static void EmitInt64ModTruncDiv(FlowGraphCompiler* compiler,
// Handle modulo/division by zero exception on slow path.
if (slow_path->has_divide_by_zero()) {
__ CompareRegisters(right, ZR);
__ b(compiler->intrinsic_mode() ? compiler->intrinsic_slow_path_label()
: slow_path->entry_label(),
EQ);
__ b(slow_path->entry_label(), EQ);
}
// Perform actual operation
@ -5679,14 +5677,12 @@ static void EmitInt64ModTruncDiv(FlowGraphCompiler* compiler,
// For the % operator, the sdiv instruction does not
// quite do what we want. Adjust for sign on slow path.
__ CompareRegisters(out, ZR);
__ b(compiler->intrinsic_mode() ? compiler->intrinsic_slow_path_label()
: slow_path->adjust_sign_label(),
LT);
__ b(slow_path->adjust_sign_label(), LT);
} else {
__ sdiv(out, left, right);
}
if (!compiler->intrinsic_mode() && slow_path->is_needed()) {
if (slow_path->is_needed()) {
__ Bind(slow_path->exit_label());
compiler->AddSlowPathCode(slow_path);
}

View file

@ -6117,18 +6117,14 @@ static void EmitInt64ModTruncDiv(FlowGraphCompiler* compiler,
// Handle modulo/division by zero exception on slow path.
if (slow_path->has_divide_by_zero()) {
__ testq(right, right);
__ j(EQUAL, compiler->intrinsic_mode()
? compiler->intrinsic_slow_path_label()
: slow_path->entry_label());
__ j(EQUAL, slow_path->entry_label());
}
// Handle modulo/division by minus one explicitly on slow path
// (to avoid arithmetic exception on 0x8000000000000000 / -1).
if (slow_path->has_divide_by_minus_one()) {
__ cmpq(right, compiler::Immediate(-1));
__ j(EQUAL, compiler->intrinsic_mode()
? compiler->intrinsic_slow_path_label()
: slow_path->div_by_minus_one_label());
__ j(EQUAL, slow_path->div_by_minus_one_label());
}
// Perform actual operation
@ -6167,15 +6163,13 @@ static void EmitInt64ModTruncDiv(FlowGraphCompiler* compiler,
// For the % operator, again the idiv instruction does
// not quite do what we want. Adjust for sign on slow path.
__ testq(out, out);
__ j(LESS, compiler->intrinsic_mode()
? compiler->intrinsic_slow_path_label()
: slow_path->adjust_sign_label());
__ j(LESS, slow_path->adjust_sign_label());
} else {
ASSERT(out == RAX);
ASSERT(tmp == RDX);
}
if (!compiler->intrinsic_mode() && slow_path->is_needed()) {
if (slow_path->is_needed()) {
__ Bind(slow_path->exit_label());
compiler->AddSlowPathCode(slow_path);
}

View file

@ -201,19 +201,6 @@ static Definition* CreateBoxedParameterIfNeeded(BlockBuilder* builder,
}
}
static Definition* CreateUnboxedParameterIfNeeded(BlockBuilder* builder,
Definition* value,
Representation representation,
intptr_t arg_index) {
const auto& function = builder->function();
if (!function.is_unboxed_parameter_at(arg_index)) {
return builder->AddUnboxInstr(representation, new Value(value),
/* is_checked = */ false);
} else {
return value;
}
}
static Definition* CreateBoxedResultIfNeeded(BlockBuilder* builder,
Definition* value,
Representation representation) {
@ -933,129 +920,96 @@ bool GraphIntrinsifier::Build_GrowableArraySetLength(FlowGraph* flow_graph) {
return true;
}
static bool BuildUnaryIntegerOp(FlowGraph* flow_graph, Token::Kind op_kind) {
static bool BuildUnarySmiOp(FlowGraph* flow_graph, Token::Kind op_kind) {
ASSERT(!flow_graph->function().has_unboxed_return());
ASSERT(!flow_graph->function().is_unboxed_parameter_at(0));
GraphEntryInstr* graph_entry = flow_graph->graph_entry();
auto normal_entry = graph_entry->normal_entry();
BlockBuilder builder(flow_graph, normal_entry);
Definition* left = builder.AddParameter(0, /*with_frame=*/false);
Definition* result;
if (flow_graph->function().HasUnboxedParameters() ||
flow_graph->function().HasUnboxedReturnValue()) {
left = CreateUnboxedParameterIfNeeded(&builder, left, kUnboxedInt64, 0);
result = builder.AddDefinition(
new UnaryInt64OpInstr(op_kind, new Value(left), DeoptId::kNone));
result = CreateBoxedResultIfNeeded(&builder, result, kUnboxedInt64);
} else {
builder.AddInstruction(
new CheckSmiInstr(new Value(left), DeoptId::kNone, builder.Source()));
result = builder.AddDefinition(
new UnarySmiOpInstr(op_kind, new Value(left), DeoptId::kNone));
}
builder.AddInstruction(
new CheckSmiInstr(new Value(left), DeoptId::kNone, builder.Source()));
Definition* result = builder.AddDefinition(
new UnarySmiOpInstr(op_kind, new Value(left), DeoptId::kNone));
builder.AddReturn(new Value(result));
return true;
}
bool GraphIntrinsifier::Build_Smi_bitNegate(FlowGraph* flow_graph) {
return BuildUnaryIntegerOp(flow_graph, Token::kBIT_NOT);
return BuildUnarySmiOp(flow_graph, Token::kBIT_NOT);
}
bool GraphIntrinsifier::Build_Integer_negate(FlowGraph* flow_graph) {
return BuildUnaryIntegerOp(flow_graph, Token::kNEGATE);
return BuildUnarySmiOp(flow_graph, Token::kNEGATE);
}
static bool BuildBinaryIntegerOp(FlowGraph* flow_graph,
Token::Kind op_kind,
bool force_boxed = false) {
static bool BuildBinarySmiOp(FlowGraph* flow_graph, Token::Kind op_kind) {
ASSERT(!flow_graph->function().has_unboxed_return());
ASSERT(!flow_graph->function().is_unboxed_parameter_at(0));
ASSERT(!flow_graph->function().is_unboxed_parameter_at(1));
GraphEntryInstr* graph_entry = flow_graph->graph_entry();
auto normal_entry = graph_entry->normal_entry();
BlockBuilder builder(flow_graph, normal_entry);
Definition* left = builder.AddParameter(0, /*with_frame=*/false);
Definition* right = builder.AddParameter(1, /*with_frame=*/false);
Definition* result;
if (!force_boxed && (flow_graph->function().HasUnboxedParameters() ||
flow_graph->function().HasUnboxedReturnValue())) {
left = CreateUnboxedParameterIfNeeded(&builder, left, kUnboxedInt64, 0);
right = CreateUnboxedParameterIfNeeded(&builder, right, kUnboxedInt64, 1);
switch (op_kind) {
case Token::kSHL:
case Token::kSHR:
case Token::kUSHR:
result = builder.AddDefinition(new ShiftInt64OpInstr(
op_kind, new Value(left), new Value(right), DeoptId::kNone));
break;
default:
result = builder.AddDefinition(new BinaryInt64OpInstr(
op_kind, new Value(left), new Value(right), DeoptId::kNone));
break;
}
result = CreateBoxedResultIfNeeded(&builder, result, kUnboxedInt64);
} else {
left = CreateBoxedParameterIfNeeded(&builder, left, kUnboxedInt64, 0);
right = CreateBoxedParameterIfNeeded(&builder, right, kUnboxedInt64, 1);
builder.AddInstruction(
new CheckSmiInstr(new Value(left), DeoptId::kNone, builder.Source()));
builder.AddInstruction(
new CheckSmiInstr(new Value(right), DeoptId::kNone, builder.Source()));
result = builder.AddDefinition(new BinarySmiOpInstr(
op_kind, new Value(left), new Value(right), DeoptId::kNone));
result = CreateUnboxedResultIfNeeded(&builder, result);
}
builder.AddInstruction(
new CheckSmiInstr(new Value(left), DeoptId::kNone, builder.Source()));
builder.AddInstruction(
new CheckSmiInstr(new Value(right), DeoptId::kNone, builder.Source()));
Definition* result = builder.AddDefinition(new BinarySmiOpInstr(
op_kind, new Value(left), new Value(right), DeoptId::kNone));
builder.AddReturn(new Value(result));
return true;
}
bool GraphIntrinsifier::Build_Integer_add(FlowGraph* flow_graph) {
return BuildBinaryIntegerOp(flow_graph, Token::kADD);
return BuildBinarySmiOp(flow_graph, Token::kADD);
}
bool GraphIntrinsifier::Build_Integer_sub(FlowGraph* flow_graph) {
return BuildBinaryIntegerOp(flow_graph, Token::kSUB);
return BuildBinarySmiOp(flow_graph, Token::kSUB);
}
bool GraphIntrinsifier::Build_Integer_mul(FlowGraph* flow_graph) {
return BuildBinaryIntegerOp(flow_graph, Token::kMUL);
return BuildBinarySmiOp(flow_graph, Token::kMUL);
}
bool GraphIntrinsifier::Build_Integer_mod(FlowGraph* flow_graph) {
bool force_boxed = false;
#if defined(TARGET_ARCH_ARM)
if (!TargetCPUFeatures::can_divide()) {
return false;
}
force_boxed = true; // BinaryInt64Op(kMOD) not implemented
#endif
return BuildBinaryIntegerOp(flow_graph, Token::kMOD, force_boxed);
return BuildBinarySmiOp(flow_graph, Token::kMOD);
}
bool GraphIntrinsifier::Build_Integer_truncDivide(FlowGraph* flow_graph) {
bool force_boxed = false;
#if defined(TARGET_ARCH_ARM)
if (!TargetCPUFeatures::can_divide()) {
return false;
}
force_boxed = true; // BinaryInt64Op(kTRUNCDIV) not implemented
#endif
return BuildBinaryIntegerOp(flow_graph, Token::kTRUNCDIV, force_boxed);
return BuildBinarySmiOp(flow_graph, Token::kTRUNCDIV);
}
bool GraphIntrinsifier::Build_Integer_bitAnd(FlowGraph* flow_graph) {
return BuildBinaryIntegerOp(flow_graph, Token::kBIT_AND);
return BuildBinarySmiOp(flow_graph, Token::kBIT_AND);
}
bool GraphIntrinsifier::Build_Integer_bitOr(FlowGraph* flow_graph) {
return BuildBinaryIntegerOp(flow_graph, Token::kBIT_OR);
return BuildBinarySmiOp(flow_graph, Token::kBIT_OR);
}
bool GraphIntrinsifier::Build_Integer_bitXor(FlowGraph* flow_graph) {
return BuildBinaryIntegerOp(flow_graph, Token::kBIT_XOR);
return BuildBinarySmiOp(flow_graph, Token::kBIT_XOR);
}
bool GraphIntrinsifier::Build_Integer_sar(FlowGraph* flow_graph) {
return BuildBinaryIntegerOp(flow_graph, Token::kSHR);
return BuildBinarySmiOp(flow_graph, Token::kSHR);
}
bool GraphIntrinsifier::Build_Integer_shr(FlowGraph* flow_graph) {
return BuildBinaryIntegerOp(flow_graph, Token::kUSHR);
return BuildBinarySmiOp(flow_graph, Token::kUSHR);
}
static Definition* ConvertOrUnboxDoubleParameter(BlockBuilder* builder,

View file

@ -264,7 +264,7 @@ namespace dart {
V(_IntegerImplementation, <, Integer_lessThan, 0x39643178) \
V(_IntegerImplementation, <=, Integer_lessEqualThan, 0x73d2a9f5) \
V(_IntegerImplementation, >=, Integer_greaterEqualThan, 0xbc280c13) \
V(_IntegerImplementation, <<, Integer_shl, 0x766f00e5) \
V(_IntegerImplementation, <<, Integer_shl, 0x766f04a6) \
V(_Double, toInt, DoubleToInteger, 0x676f1ce8) \
#define MATH_LIB_INTRINSIC_LIST(V) \
@ -352,18 +352,18 @@ namespace dart {
0x17f90910) \
V(_ExternalTwoByteString, codeUnitAt, ExternalTwoByteStringCodeUnitAt, \
0x17f90910) \
V(_Smi, ~, Smi_bitNegate, 0x8254f51b) \
V(_IntegerImplementation, +, Integer_add, 0xd561008f) \
V(_IntegerImplementation, -, Integer_sub, 0xc96a0f80) \
V(_IntegerImplementation, *, Integer_mul, 0xacd9641d) \
V(_IntegerImplementation, %, Integer_mod, 0xfcf7cc13) \
V(_IntegerImplementation, ~/, Integer_truncDivide, 0xdda49e7f) \
V(_IntegerImplementation, unary-, Integer_negate, 0xf7a9a696) \
V(_IntegerImplementation, &, Integer_bitAnd, 0x8b9d7c33) \
V(_IntegerImplementation, |, Integer_bitOr, 0x8f47f5eb) \
V(_IntegerImplementation, ^, Integer_bitXor, 0xd838bef2) \
V(_IntegerImplementation, >>, Integer_sar, 0x931fbb8a) \
V(_IntegerImplementation, >>>, Integer_shr, 0x7495f7ec) \
V(_Smi, ~, Smi_bitNegate, 0x8254f8dc) \
V(_IntegerImplementation, +, Integer_add, 0xd5610450) \
V(_IntegerImplementation, -, Integer_sub, 0xc96a1341) \
V(_IntegerImplementation, *, Integer_mul, 0xacd967de) \
V(_IntegerImplementation, %, Integer_mod, 0xfcf7cfd4) \
V(_IntegerImplementation, ~/, Integer_truncDivide, 0xdda4a240) \
V(_IntegerImplementation, unary-, Integer_negate, 0xf7a9aa57) \
V(_IntegerImplementation, &, Integer_bitAnd, 0x8b9d7ff4) \
V(_IntegerImplementation, |, Integer_bitOr, 0x8f47f9ac) \
V(_IntegerImplementation, ^, Integer_bitXor, 0xd838c2b3) \
V(_IntegerImplementation, >>, Integer_sar, 0x931fbf4b) \
V(_IntegerImplementation, >>>, Integer_shr, 0x7495fbad) \
V(_Double, unary-, DoubleFlipSignBit, 0x3d39082b) \
V(_Double, truncateToDouble, DoubleTruncate, 0x62d48298) \
V(_Double, roundToDouble, DoubleRound, 0x5649c63f) \

View file

@ -8,19 +8,23 @@ abstract class _IntegerImplementation implements int {
@pragma("vm:recognized", "graph-intrinsic")
@pragma("vm:non-nullable-result-type")
@pragma("vm:never-inline")
@pragma("vm:disable-unboxed-parameters")
num operator +(num other) => other._addFromInteger(this);
@pragma("vm:recognized", "graph-intrinsic")
@pragma("vm:non-nullable-result-type")
@pragma("vm:never-inline")
@pragma("vm:disable-unboxed-parameters")
num operator -(num other) => other._subFromInteger(this);
@pragma("vm:recognized", "graph-intrinsic")
@pragma("vm:non-nullable-result-type")
@pragma("vm:never-inline")
@pragma("vm:disable-unboxed-parameters")
num operator *(num other) => other._mulFromInteger(this);
@pragma("vm:recognized", "graph-intrinsic")
@pragma("vm:non-nullable-result-type")
@pragma("vm:never-inline")
@pragma("vm:disable-unboxed-parameters")
int operator ~/(num other) {
if ((other is int) && (other == 0)) {
throw const IntegerDivisionByZeroException();
@ -35,6 +39,7 @@ abstract class _IntegerImplementation implements int {
@pragma("vm:recognized", "graph-intrinsic")
@pragma("vm:non-nullable-result-type")
@pragma("vm:never-inline")
@pragma("vm:disable-unboxed-parameters")
num operator %(num other) {
if ((other is int) && (other == 0)) {
throw const IntegerDivisionByZeroException();
@ -45,6 +50,7 @@ abstract class _IntegerImplementation implements int {
@pragma("vm:recognized", "graph-intrinsic")
@pragma("vm:non-nullable-result-type")
@pragma("vm:never-inline")
@pragma("vm:disable-unboxed-parameters")
int operator -() {
// Issue(https://dartbug.com/39639): The analyzer incorrectly reports the
// result type as `num`.
@ -54,14 +60,17 @@ abstract class _IntegerImplementation implements int {
@pragma("vm:recognized", "graph-intrinsic")
@pragma("vm:non-nullable-result-type")
@pragma("vm:never-inline")
@pragma("vm:disable-unboxed-parameters")
int operator &(int other) => other._bitAndFromInteger(this);
@pragma("vm:recognized", "graph-intrinsic")
@pragma("vm:non-nullable-result-type")
@pragma("vm:never-inline")
@pragma("vm:disable-unboxed-parameters")
int operator |(int other) => other._bitOrFromInteger(this);
@pragma("vm:recognized", "graph-intrinsic")
@pragma("vm:non-nullable-result-type")
@pragma("vm:never-inline")
@pragma("vm:disable-unboxed-parameters")
int operator ^(int other) => other._bitXorFromInteger(this);
num remainder(num other) {
@ -99,14 +108,17 @@ abstract class _IntegerImplementation implements int {
@pragma("vm:recognized", "graph-intrinsic")
@pragma("vm:non-nullable-result-type")
@pragma("vm:never-inline")
@pragma("vm:disable-unboxed-parameters")
int operator >>(int other) => other._shrFromInteger(this);
@pragma("vm:recognized", "graph-intrinsic")
@pragma("vm:non-nullable-result-type")
@pragma("vm:never-inline")
@pragma("vm:disable-unboxed-parameters")
int operator >>>(int other) => other._ushrFromInteger(this);
@pragma("vm:recognized", "asm-intrinsic")
@pragma("vm:non-nullable-result-type")
@pragma("vm:never-inline")
@pragma("vm:disable-unboxed-parameters")
int operator <<(int other) => other._shlFromInteger(this);
@pragma("vm:recognized", "asm-intrinsic")
@ -544,6 +556,7 @@ class _Smi extends _IntegerImplementation {
int get _identityHashCode => this;
@pragma("vm:recognized", "graph-intrinsic")
@pragma("vm:exact-result-type", "dart:core#_Smi")
@pragma("vm:disable-unboxed-parameters")
int operator ~() native "Smi_bitNegate";
@pragma("vm:recognized", "asm-intrinsic")
@pragma("vm:exact-result-type", "dart:core#_Smi")