From 5fd6c8a3c19b062887b5830b37bfeb2347eb4d17 Mon Sep 17 00:00:00 2001 From: Daco Harkes Date: Wed, 23 Oct 2019 12:59:28 +0000 Subject: [PATCH] [vm] Do not fuse constants with different representations Issue: https://github.com/dart-lang/sdk/issues/39044 Change-Id: I19b1309adb769742b498ed0b6fe80ae38a779405 Cq-Include-Trybots: luci.dart.try:vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64-try,app-kernel-linux-debug-x64-try,vm-kernel-linux-debug-ia32-try,vm-dartkb-linux-debug-simarm64-try,vm-kernel-win-debug-x64-try,vm-kernel-win-debug-ia32-try,vm-dartkb-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-dartkb-linux-release-x64-abi-try,vm-kernel-precomp-android-release-arm64-try,vm-kernel-asan-linux-release-x64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-simarm64-try,vm-kernel-precomp-android-release-arm_x64-try,vm-kernel-precomp-obfuscate-linux-release-x64-try,vm-kernel-reload-rollback-linux-debug-x64-try,vm-kernel-precomp-mac-release-simarm_x64-try,dart-sdk-linux-try,flutter-engine-linux-try,analyzer-analysis-server-linux-try,analyzer-linux-release-try,front-end-linux-release-x64-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/122396 Commit-Queue: Daco Harkes Reviewed-by: Martin Kustermann --- runtime/bin/ffi_test/ffi_test_functions.cc | 8 +++++++ runtime/vm/compiler/backend/il.cc | 3 ++- tests/ffi/regress_39044_test.dart | 28 ++++++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 tests/ffi/regress_39044_test.dart diff --git a/runtime/bin/ffi_test/ffi_test_functions.cc b/runtime/bin/ffi_test/ffi_test_functions.cc index 62c349b29a6..d0f4db35905 100644 --- a/runtime/bin/ffi_test/ffi_test_functions.cc +++ b/runtime/bin/ffi_test/ffi_test_functions.cc @@ -123,6 +123,14 @@ DART_EXPORT int64_t IntComputation(int8_t a, int16_t b, int32_t c, int64_t d) { return retval; } +// Used in regress_39044_test.dart. +DART_EXPORT int64_t Regress39044(int64_t a, int8_t b) { + std::cout << "Regress39044(" << a << ", " << static_cast(b) << ")\n"; + const int64_t retval = a - b; + std::cout << "returning " << retval << "\n"; + return retval; +} + // Performs some computation on various sized unsigned ints. // Used for testing value ranges for unsigned ints. DART_EXPORT int64_t UintComputation(uint8_t a, diff --git a/runtime/vm/compiler/backend/il.cc b/runtime/vm/compiler/backend/il.cc index b5f807aeb65..981737f240a 100644 --- a/runtime/vm/compiler/backend/il.cc +++ b/runtime/vm/compiler/backend/il.cc @@ -1117,7 +1117,8 @@ ConstantInstr::ConstantInstr(const Object& value, TokenPosition token_pos) bool ConstantInstr::AttributesEqual(Instruction* other) const { ConstantInstr* other_constant = other->AsConstant(); ASSERT(other_constant != NULL); - return (value().raw() == other_constant->value().raw()); + return (value().raw() == other_constant->value().raw() && + representation() == other_constant->representation()); } UnboxedConstantInstr::UnboxedConstantInstr(const Object& value, diff --git a/tests/ffi/regress_39044_test.dart b/tests/ffi/regress_39044_test.dart new file mode 100644 index 00000000000..785500dab74 --- /dev/null +++ b/tests/ffi/regress_39044_test.dart @@ -0,0 +1,28 @@ +// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +// +// Check that the optimizer does not fuse constants with different +// representations. +// +// SharedObjects=ffi_test_functions + +import "dart:ffi"; + +import "package:expect/expect.dart"; + +import "dylib_utils.dart"; + +main() { + final ffiTestFunctions = dlopenPlatformSpecific("ffi_test_functions"); + + final intComputation = ffiTestFunctions.lookupFunction< + Int64 Function(Int64, Int8), int Function(int, int)>("Regress39044"); + + // The arguments are the same Smi constant, however they are different sizes. + final result = intComputation( + /* dart::kUnboxedInt64 --> int64_t */ 1, + /* dart::kUnboxedInt32 --> truncated to int8_t */ 1); + + Expect.equals(0, result); +}