From 37d2c9408e43f3e13e4a64e01f7af4ddc6e6bdbc Mon Sep 17 00:00:00 2001 From: Andreas Haas Date: Wed, 21 Aug 2019 14:50:05 +0200 Subject: [PATCH] [turbofan][cleanup] Fix LinkageLocation equality The LinkageLocation currently consists of two fields, a bit_field and a machine_type. The existing equality check only checked the equality of the bit_field, which meant that a FP register location and a GP register location could alias. I added a static {IsSameLocation} function which checks that not just the bit_field but also if one of the two locations at least has a subtype of the other. Note that we do not check for type-equality because {CanTailCall} checks, which are the main user of the LinkageLocation equality check, should pass even if the result types are in a sub-typing relationship. R=mstarzinger@chromium.org Bug: v8:9396 Change-Id: Iaa2d11311d0c18e8ffc1dd934e369106ab2456a6 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1763533 Commit-Queue: Andreas Haas Reviewed-by: Michael Starzinger Cr-Commit-Position: refs/heads/master@{#63319} --- src/compiler/linkage.cc | 17 +++++++---------- src/compiler/linkage.h | 18 +++++++++++++++--- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/compiler/linkage.cc b/src/compiler/linkage.cc index cf3b7b84a1..39c93c0328 100644 --- a/src/compiler/linkage.cc +++ b/src/compiler/linkage.cc @@ -73,15 +73,6 @@ MachineSignature* CallDescriptor::GetMachineSignature(Zone* zone) const { return new (zone) MachineSignature(return_count, param_count, types); } -bool CallDescriptor::HasSameReturnLocationsAs( - const CallDescriptor* other) const { - if (ReturnCount() != other->ReturnCount()) return false; - for (size_t i = 0; i < ReturnCount(); ++i) { - if (GetReturnLocation(i) != other->GetReturnLocation(i)) return false; - } - return true; -} - int CallDescriptor::GetFirstUnusedStackSlot() const { int slots_above_sp = 0; for (size_t i = 0; i < InputCount(); ++i) { @@ -129,7 +120,13 @@ int CallDescriptor::GetTaggedParameterSlots() const { } bool CallDescriptor::CanTailCall(const CallDescriptor* callee) const { - return HasSameReturnLocationsAs(callee); + if (ReturnCount() != callee->ReturnCount()) return false; + for (size_t i = 0; i < ReturnCount(); ++i) { + if (!LinkageLocation::IsSameLocation(GetReturnLocation(i), + callee->GetReturnLocation(i))) + return false; + } + return true; } // TODO(jkummerow, sigurds): Arguably frame size calculation should be diff --git a/src/compiler/linkage.h b/src/compiler/linkage.h index 83a052b775..69e7fbfa42 100644 --- a/src/compiler/linkage.h +++ b/src/compiler/linkage.h @@ -34,13 +34,27 @@ class OsrHelper; class LinkageLocation { public: bool operator==(const LinkageLocation& other) const { - return bit_field_ == other.bit_field_; + return bit_field_ == other.bit_field_ && + machine_type_ == other.machine_type_; } bool operator!=(const LinkageLocation& other) const { return !(*this == other); } + static bool IsSameLocation(const LinkageLocation& a, + const LinkageLocation& b) { + // Different MachineTypes may end up at the same physical location. With the + // sub-type check we make sure that types like {AnyTagged} and + // {TaggedPointer} which would end up with the same physical location are + // considered equal here. + return (a.bit_field_ == b.bit_field_) && + (IsSubtype(a.machine_type_.representation(), + b.machine_type_.representation()) || + IsSubtype(b.machine_type_.representation(), + a.machine_type_.representation())); + } + static LinkageLocation ForAnyRegister( MachineType type = MachineType::None()) { return LinkageLocation(REGISTER, ANY_REGISTER, type); @@ -317,8 +331,6 @@ class V8_EXPORT_PRIVATE CallDescriptor final bool UsesOnlyRegisters() const; - bool HasSameReturnLocationsAs(const CallDescriptor* other) const; - // Returns the first stack slot that is not used by the stack parameters. int GetFirstUnusedStackSlot() const;