From 57e5a66c4fcd62e229c83e02a67f52b788fbf2d1 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 5 May 2014 15:02:01 -0700 Subject: [PATCH] Backed out changeset 695049af7654 for having the wrong bug number. --- js/src/jit/MIR.cpp | 4 +- js/src/jit/MIR.h | 136 ++++++++---------- js/src/jit/RangeAnalysis.cpp | 265 +++++++++++++++++------------------ 3 files changed, 185 insertions(+), 220 deletions(-) diff --git a/js/src/jit/MIR.cpp b/js/src/jit/MIR.cpp index 79587451e6e4..9bb99a44c2ec 100644 --- a/js/src/jit/MIR.cpp +++ b/js/src/jit/MIR.cpp @@ -1621,7 +1621,7 @@ MAdd::fallible() const { // the add is fallible if range analysis does not say that it is finite, AND // either the truncation analysis shows that there are non-truncated uses. - if (truncateKind() >= IndirectTruncate) + if (isTruncated()) return false; if (range() && range()->hasInt32Bounds()) return false; @@ -1632,7 +1632,7 @@ bool MSub::fallible() const { // see comment in MAdd::fallible() - if (truncateKind() >= IndirectTruncate) + if (isTruncated()) return false; if (range() && range()->hasInt32Bounds()) return false; diff --git a/js/src/jit/MIR.h b/js/src/jit/MIR.h index 3326084c3a20..1a434b88e42f 100644 --- a/js/src/jit/MIR.h +++ b/js/src/jit/MIR.h @@ -412,44 +412,8 @@ class MDefinition : public MNode virtual void analyzeEdgeCasesForward(); virtual void analyzeEdgeCasesBackward(); - // When a floating-point value is used by nodes which would prefer to - // recieve integer inputs, we may be able to help by computing our result - // into an integer directly. - // - // A value can be truncated in 4 differents ways: - // 1. Ignore Infinities (x / 0 --> 0). - // 2. Ignore overflow (INT_MIN / -1 == (INT_MAX + 1) --> INT_MIN) - // 3. Ignore negative zeros. (-0 --> 0) - // 4. Ignore remainder. (3 / 4 --> 0) - // - // Indirect truncation is used to represent that we are interested in the - // truncated result, but only if it can safely flow into operations which - // are computed modulo 2^32, such as (2) and (3). Infinities are not safe, - // as they would have absorbed other math operations. Remainders are not - // safe, as fractions can be scaled up by multiplication. - // - // Division is a particularly interesting node here because it covers all 4 - // cases even when its own operands are integers. - // - // Note that these enum values are ordered from least value-modifying to - // most value-modifying, and code relies on this ordering. - enum TruncateKind { - // No correction. - NoTruncate = 0, - // An integer is desired, but we can't skip bailout checks. - TruncateAfterBailouts = 1, - // The value will be truncated after some arithmetic (see above). - IndirectTruncate = 2, - // Direct and infallible truncation to int32. - Truncate = 3 - }; - - // Apply the given truncate to this node itself. - virtual bool truncate(TruncateKind kind); - - // Determine what kind of truncate this node prefers for the operand at the - // given index. - virtual TruncateKind operandTruncateKind(size_t index) const; + virtual bool truncate(); + virtual bool isOperandTruncated(size_t index) const; // Compute an absolute or symbolic range for the value of this node. virtual void computeRange(TempAllocator &alloc) { @@ -1013,7 +977,7 @@ class MConstant : public MNullaryInstruction } void computeRange(TempAllocator &alloc); - bool truncate(TruncateKind kind); + bool truncate(); bool canProduceFloat32() const; }; @@ -2411,8 +2375,8 @@ class MCompare void trySpecializeFloat32(TempAllocator &alloc); bool isFloat32Commutative() const { return true; } - bool truncate(TruncateKind kind); - TruncateKind operandTruncateKind(size_t index) const; + bool truncate(); + bool isOperandTruncated(size_t index) const; # ifdef DEBUG bool isConsistentFloat32Use(MUse *use) const { @@ -2941,10 +2905,8 @@ class MToDouble private: ConversionKind conversion_; - TruncateKind implicitTruncate_; - MToDouble(MDefinition *def, ConversionKind conversion = NonStringPrimitives) - : MUnaryInstruction(def), conversion_(conversion), implicitTruncate_(NoTruncate) + : MUnaryInstruction(def), conversion_(conversion) { setResultType(MIRType_Double); setMovable(); @@ -2984,19 +2946,12 @@ class MToDouble } void computeRange(TempAllocator &alloc); - bool truncate(TruncateKind kind); - TruncateKind operandTruncateKind(size_t index) const; + bool truncate(); + bool isOperandTruncated(size_t index) const; #ifdef DEBUG bool isConsistentFloat32Use(MUse *use) const { return true; } #endif - - TruncateKind truncateKind() const { - return implicitTruncate_; - } - void setTruncateKind(TruncateKind kind) { - implicitTruncate_ = Max(implicitTruncate_, kind); - } }; // Converts a primitive (either typed or untyped) to a float32. If the input is @@ -3216,7 +3171,7 @@ class MTruncateToInt32 : public MUnaryInstruction } void computeRange(TempAllocator &alloc); - TruncateKind operandTruncateKind(size_t index) const; + bool isOperandTruncated(size_t index) const; # ifdef DEBUG bool isConsistentFloat32Use(MUse *use) const { return true; @@ -3394,7 +3349,7 @@ class MBinaryBitwiseInstruction return AliasSet::None(); } - TruncateKind operandTruncateKind(size_t index) const; + bool isOperandTruncated(size_t index) const; }; class MBitAnd : public MBinaryBitwiseInstruction @@ -3569,14 +3524,14 @@ class MBinaryArithInstruction // This optimization happens when the multiplication cannot be truncated // even if all uses are truncating its result, such as when the range // analysis detect a precision loss in the multiplication. - TruncateKind implicitTruncate_; + bool implicitTruncate_; void inferFallback(BaselineInspector *inspector, jsbytecode *pc); public: MBinaryArithInstruction(MDefinition *left, MDefinition *right) : MBinaryInstruction(left, right), - implicitTruncate_(NoTruncate) + implicitTruncate_(false) { setMovable(); } @@ -3611,13 +3566,10 @@ class MBinaryArithInstruction } bool isTruncated() const { - return implicitTruncate_ == Truncate; - } - TruncateKind truncateKind() const { return implicitTruncate_; } - void setTruncateKind(TruncateKind kind) { - implicitTruncate_ = Max(implicitTruncate_, kind); + void setTruncated(bool truncate) { + implicitTruncate_ = truncate; } }; @@ -4065,7 +4017,7 @@ class MAdd : public MBinaryArithInstruction add->specialization_ = type; add->setResultType(type); if (type == MIRType_Int32) { - add->setTruncateKind(Truncate); + add->setTruncated(true); add->setCommutative(); } return add; @@ -4079,8 +4031,8 @@ class MAdd : public MBinaryArithInstruction bool fallible() const; void computeRange(TempAllocator &alloc); - bool truncate(TruncateKind kind); - TruncateKind operandTruncateKind(size_t index) const; + bool truncate(); + bool isOperandTruncated(size_t index) const; bool writeRecoverData(CompactBufferWriter &writer) const; bool canRecoverOnBailout() const { @@ -4108,7 +4060,7 @@ class MSub : public MBinaryArithInstruction sub->specialization_ = type; sub->setResultType(type); if (type == MIRType_Int32) - sub->setTruncateKind(Truncate); + sub->setTruncated(true); return sub; } @@ -4120,8 +4072,8 @@ class MSub : public MBinaryArithInstruction bool fallible() const; void computeRange(TempAllocator &alloc); - bool truncate(TruncateKind kind); - TruncateKind operandTruncateKind(size_t index) const; + bool truncate(); + bool isOperandTruncated(size_t index) const; }; class MMul : public MBinaryArithInstruction @@ -4148,7 +4100,7 @@ class MMul : public MBinaryArithInstruction // This implements the required behavior for Math.imul, which // can never fail and always truncates its output to int32. canBeNegativeZero_ = false; - setTruncateKind(Truncate); + setTruncated(true); setCommutative(); } JS_ASSERT_IF(mode != Integer, mode == Normal); @@ -4213,8 +4165,8 @@ class MMul : public MBinaryArithInstruction bool isFloat32Commutative() const { return true; } void computeRange(TempAllocator &alloc); - bool truncate(TruncateKind kind); - TruncateKind operandTruncateKind(size_t index) const; + bool truncate(); + bool isOperandTruncated(size_t index) const; Mode mode() const { return mode_; } }; @@ -4227,13 +4179,32 @@ class MDiv : public MBinaryArithInstruction bool canBeNegativeDividend_; bool unsigned_; + // A Division can be truncated in 4 differents ways: + // 1. Ignore Infinities (x / 0 --> 0). + // 2. Ignore overflow (INT_MIN / -1 == (INT_MAX + 1) --> INT_MIN) + // 3. Ignore negative zeros. (-0 --> 0) + // 4. Ignore remainder. (3 / 4 --> 0) + // + // isTruncatedIndirectly is used to represent that we are interested in the + // truncated result, but only if they it can safely flow in operations which + // are computed modulo 2^32, such as (2) and (3). + // + // A division can return either Infinities (1) or a remainder (4) when both + // operands are integers. Infinities are not safe, as they would have + // absorbed other math operations. Remainders are not safe, as multiple can + // add up to integers. This implies that we need to distinguish between a + // division which is truncated directly (isTruncated) or which flow into + // truncated operations (isTruncatedIndirectly). + bool isTruncatedIndirectly_; + MDiv(MDefinition *left, MDefinition *right, MIRType type) : MBinaryArithInstruction(left, right), canBeNegativeZero_(true), canBeNegativeOverflow_(true), canBeDivideByZero_(true), canBeNegativeDividend_(true), - unsigned_(false) + unsigned_(false), + isTruncatedIndirectly_(false) { if (type != MIRType_Value) specialization_ = type; @@ -4254,7 +4225,7 @@ class MDiv : public MBinaryArithInstruction MDiv *div = new(alloc) MDiv(left, right, type); div->unsigned_ = unsignd; if (type == MIRType_Int32) - div->setTruncateKind(Truncate); + div->setTruncated(true); return div; } @@ -4290,7 +4261,10 @@ class MDiv : public MBinaryArithInstruction } bool isTruncatedIndirectly() const { - return truncateKind() >= IndirectTruncate; + return isTruncatedIndirectly_; + } + void setTruncatedIndirectly(bool truncate) { + isTruncatedIndirectly_ = truncate; } bool canTruncateInfinities() const { @@ -4310,7 +4284,7 @@ class MDiv : public MBinaryArithInstruction void computeRange(TempAllocator &alloc); bool fallible() const; - bool truncate(TruncateKind kind); + bool truncate(); void collectRangeInfoPreTrunc(); }; @@ -4340,7 +4314,7 @@ class MMod : public MBinaryArithInstruction MMod *mod = new(alloc) MMod(left, right, type); mod->unsigned_ = unsignd; if (type == MIRType_Int32) - mod->setTruncateKind(Truncate); + mod->setTruncated(true); return mod; } @@ -4364,7 +4338,7 @@ class MMod : public MBinaryArithInstruction bool fallible() const; void computeRange(TempAllocator &alloc); - bool truncate(TruncateKind kind); + bool truncate(); void collectRangeInfoPreTrunc(); }; @@ -6569,7 +6543,7 @@ class MLoadTypedArrayElementStatic } void computeRange(TempAllocator &alloc); - bool truncate(TruncateKind kind); + bool truncate(); bool canProduceFloat32() const { return typedArray_->type() == ScalarTypeDescr::TYPE_FLOAT32; } }; @@ -6634,7 +6608,7 @@ class MStoreTypedArrayElement void setRacy() { racy_ = true; } - TruncateKind operandTruncateKind(size_t index) const; + bool isOperandTruncated(size_t index) const; bool canConsumeFloat32(MUse *use) const { return use->index() == 2 && arrayType_ == ScalarTypeDescr::TYPE_FLOAT32; @@ -6702,7 +6676,7 @@ class MStoreTypedArrayElementHole AliasSet getAliasSet() const { return AliasSet::Store(AliasSet::TypedArrayElement); } - TruncateKind operandTruncateKind(size_t index) const; + bool isOperandTruncated(size_t index) const; bool canConsumeFloat32(MUse *use) const { return use->index() == 3 && arrayType_ == ScalarTypeDescr::TYPE_FLOAT32; @@ -6749,7 +6723,7 @@ class MStoreTypedArrayElementStatic : AliasSet getAliasSet() const { return AliasSet::Store(AliasSet::TypedArrayElement); } - TruncateKind operandTruncateKind(size_t index) const; + bool isOperandTruncated(size_t index) const; bool canConsumeFloat32(MUse *use) const { return use->index() == 1 && typedArray_->type() == ScalarTypeDescr::TYPE_FLOAT32; diff --git a/js/src/jit/RangeAnalysis.cpp b/js/src/jit/RangeAnalysis.cpp index 886965be4cf5..965387ca2733 100644 --- a/js/src/jit/RangeAnalysis.cpp +++ b/js/src/jit/RangeAnalysis.cpp @@ -2136,14 +2136,14 @@ Range::wrapAroundToBoolean() } bool -MDefinition::truncate(TruncateKind kind) +MDefinition::truncate() { // No procedure defined for truncating this instruction. return false; } bool -MConstant::truncate(TruncateKind kind) +MConstant::truncate() { if (!value_.isDouble()) return false; @@ -2158,15 +2158,15 @@ MConstant::truncate(TruncateKind kind) } bool -MAdd::truncate(TruncateKind kind) +MAdd::truncate() { // Remember analysis, needed for fallible checks. - setTruncateKind(kind); + setTruncated(true); if (type() == MIRType_Double || type() == MIRType_Int32) { specialization_ = MIRType_Int32; setResultType(MIRType_Int32); - if (kind >= IndirectTruncate && range()) + if (range()) range()->wrapAroundToInt32(); return true; } @@ -2175,15 +2175,15 @@ MAdd::truncate(TruncateKind kind) } bool -MSub::truncate(TruncateKind kind) +MSub::truncate() { // Remember analysis, needed for fallible checks. - setTruncateKind(kind); + setTruncated(true); if (type() == MIRType_Double || type() == MIRType_Int32) { specialization_ = MIRType_Int32; setResultType(MIRType_Int32); - if (kind >= IndirectTruncate && range()) + if (range()) range()->wrapAroundToInt32(); return true; } @@ -2192,19 +2192,17 @@ MSub::truncate(TruncateKind kind) } bool -MMul::truncate(TruncateKind kind) +MMul::truncate() { // Remember analysis, needed to remove negative zero checks. - setTruncateKind(kind); + setTruncated(true); if (type() == MIRType_Double || type() == MIRType_Int32) { specialization_ = MIRType_Int32; setResultType(MIRType_Int32); - if (kind >= IndirectTruncate) { - setCanBeNegativeZero(false); - if (range()) - range()->wrapAroundToInt32(); - } + setCanBeNegativeZero(false); + if (range()) + range()->wrapAroundToInt32(); return true; } @@ -2212,19 +2210,36 @@ MMul::truncate(TruncateKind kind) } bool -MDiv::truncate(TruncateKind kind) +MDiv::truncate() { - setTruncateKind(kind); + // Remember analysis, needed to remove negative zero checks. + setTruncatedIndirectly(true); - if (type() == MIRType_Double || type() == MIRType_Int32) { - specialization_ = MIRType_Int32; - setResultType(MIRType_Int32); + // Check if this division only flows in bitwise instructions. + if (!isTruncated()) { + bool allUsesExplictlyTruncate = true; + for (MUseDefIterator use(this); allUsesExplictlyTruncate && use; use++) { + switch (use.def()->op()) { + case MDefinition::Op_BitAnd: + case MDefinition::Op_BitOr: + case MDefinition::Op_BitXor: + case MDefinition::Op_Lsh: + case MDefinition::Op_Rsh: + case MDefinition::Op_Ursh: + break; + default: + allUsesExplictlyTruncate = false; + } + } - // Divisions where the lhs and rhs are unsigned and the result is - // truncated can be lowered more efficiently. - if (tryUseUnsignedOperands()) - unsigned_ = true; + if (allUsesExplictlyTruncate) + setTruncated(true); + } + // Divisions where the lhs and rhs are unsigned and the result is + // truncated can be lowered more efficiently. + if (specialization() == MIRType_Int32 && tryUseUnsignedOperands()) { + unsigned_ = true; return true; } @@ -2233,19 +2248,14 @@ MDiv::truncate(TruncateKind kind) } bool -MMod::truncate(TruncateKind kind) +MMod::truncate() { // Remember analysis, needed to remove negative zero checks. - setTruncateKind(kind); + setTruncated(true); // As for division, handle unsigned modulus with a truncated result. - if (type() == MIRType_Double || type() == MIRType_Int32) { - specialization_ = MIRType_Int32; - setResultType(MIRType_Int32); - - if (tryUseUnsignedOperands()) - unsigned_ = true; - + if (specialization() == MIRType_Int32 && tryUseUnsignedOperands()) { + unsigned_ = true; return true; } @@ -2254,103 +2264,90 @@ MMod::truncate(TruncateKind kind) } bool -MToDouble::truncate(TruncateKind kind) +MToDouble::truncate() { JS_ASSERT(type() == MIRType_Double); - setTruncateKind(kind); - // We use the return type to flag that this MToDouble should be replaced by // a MTruncateToInt32 when modifying the graph. setResultType(MIRType_Int32); - if (kind >= IndirectTruncate) { - if (range()) - range()->wrapAroundToInt32(); - } + if (range()) + range()->wrapAroundToInt32(); return true; } bool -MLoadTypedArrayElementStatic::truncate(TruncateKind kind) +MLoadTypedArrayElementStatic::truncate() { setInfallible(); return false; } -MDefinition::TruncateKind -MDefinition::operandTruncateKind(size_t index) const +bool +MDefinition::isOperandTruncated(size_t index) const { - // Generic routine: We don't know anything. - return NoTruncate; -} - -MDefinition::TruncateKind -MTruncateToInt32::operandTruncateKind(size_t index) const -{ - // This operator is an explicit truncate to int32. - return Truncate; -} - -MDefinition::TruncateKind -MBinaryBitwiseInstruction::operandTruncateKind(size_t index) const -{ - // The bitwise operators truncate to int32. - return Truncate; -} - -MDefinition::TruncateKind -MAdd::operandTruncateKind(size_t index) const -{ - // This operator is doing some arithmetic. If its result is truncated, - // it's an indirect truncate for its operands. - return Min(truncateKind(), IndirectTruncate); -} - -MDefinition::TruncateKind -MSub::operandTruncateKind(size_t index) const -{ - // See the comment in MAdd::operandTruncateKind. - return Min(truncateKind(), IndirectTruncate); -} - -MDefinition::TruncateKind -MMul::operandTruncateKind(size_t index) const -{ - // See the comment in MAdd::operandTruncateKind. - return Min(truncateKind(), IndirectTruncate); -} - -MDefinition::TruncateKind -MToDouble::operandTruncateKind(size_t index) const -{ - // MToDouble propagates its truncate kind to its operand. - return truncateKind(); -} - -MDefinition::TruncateKind -MStoreTypedArrayElement::operandTruncateKind(size_t index) const -{ - // An integer store truncates the stored value. - return index == 2 && !isFloatArray() ? Truncate : NoTruncate; -} - -MDefinition::TruncateKind -MStoreTypedArrayElementHole::operandTruncateKind(size_t index) const -{ - // An integer store truncates the stored value. - return index == 3 && !isFloatArray() ? Truncate : NoTruncate; -} - -MDefinition::TruncateKind -MStoreTypedArrayElementStatic::operandTruncateKind(size_t index) const -{ - // An integer store truncates the stored value. - return index == 1 && !isFloatArray() ? Truncate : NoTruncate; + return false; } bool -MCompare::truncate(TruncateKind kind) +MTruncateToInt32::isOperandTruncated(size_t index) const +{ + return true; +} + +bool +MBinaryBitwiseInstruction::isOperandTruncated(size_t index) const +{ + return true; +} + +bool +MAdd::isOperandTruncated(size_t index) const +{ + return isTruncated(); +} + +bool +MSub::isOperandTruncated(size_t index) const +{ + return isTruncated(); +} + +bool +MMul::isOperandTruncated(size_t index) const +{ + return isTruncated(); +} + +bool +MToDouble::isOperandTruncated(size_t index) const +{ + // The return type is used to flag that we are replacing this Double by a + // Truncate of its operand if needed. + return type() == MIRType_Int32; +} + +bool +MStoreTypedArrayElement::isOperandTruncated(size_t index) const +{ + return index == 2 && !isFloatArray(); +} + +bool +MStoreTypedArrayElementHole::isOperandTruncated(size_t index) const +{ + return index == 3 && !isFloatArray(); +} + +bool +MStoreTypedArrayElementStatic::isOperandTruncated(size_t index) const +{ + return index == 1 && !isFloatArray(); +} + +bool +MCompare::truncate() { if (!isDoubleComparison()) return false; @@ -2362,34 +2359,32 @@ MCompare::truncate(TruncateKind kind) compareType_ = Compare_Int32; - // Truncating the operands won't change their value because we don't force a - // truncation, but it will change their type, which we need because we - // now expect integer inputs. + // Truncating the operands won't change their value, but it will change + // their type, which we need because we now expect integer inputs. truncateOperands_ = true; return true; } -MDefinition::TruncateKind -MCompare::operandTruncateKind(size_t index) const +bool +MCompare::isOperandTruncated(size_t index) const { // If we're doing an int32 comparison on operands which were previously // floating-point, convert them! JS_ASSERT_IF(truncateOperands_, isInt32Comparison()); - return truncateOperands_ ? TruncateAfterBailouts : NoTruncate; + return truncateOperands_; } -// Examine all the users of |candidate| and determine the most aggressive -// truncate kind that satisfies all of them. -static MDefinition::TruncateKind -ComputeRequestedTruncateKind(MInstruction *candidate) +// Ensure that all observables uses can work with a truncated +// version of the |candidate|'s result. +static bool +AllUsesTruncate(MInstruction *candidate) { // If the value naturally produces an int32 value (before bailout checks) // that needs no conversion, we don't have to worry about resume points // seeing truncated values. bool needsConversion = !candidate->range() || !candidate->range()->isInt32(); - MDefinition::TruncateKind kind = MDefinition::Truncate; for (MUseIterator use(candidate->usesBegin()); use != candidate->usesEnd(); use++) { if (!use->consumer()->isDefinition()) { // We can only skip testing resume points, if all original uses are @@ -2398,27 +2393,24 @@ ComputeRequestedTruncateKind(MInstruction *candidate) // value, and any bailout with a truncated value might lead an // incorrect value. if (candidate->isUseRemoved() && needsConversion) - kind = Min(kind, MDefinition::TruncateAfterBailouts); + return false; continue; } - MDefinition *consumer = use->consumer()->toDefinition(); - MDefinition::TruncateKind consumerKind = consumer->operandTruncateKind(use->index()); - kind = Min(kind, consumerKind); - if (kind == MDefinition::NoTruncate) - break; + if (!use->consumer()->toDefinition()->isOperandTruncated(use->index())) + return false; } - return kind; + return true; } -static MDefinition::TruncateKind -ComputeTruncateKind(MInstruction *candidate) +static bool +CanTruncate(MInstruction *candidate) { // Compare operations might coerce its inputs to int32 if the ranges are // correct. So we do not need to check if all uses are coerced. if (candidate->isCompare()) - return MDefinition::TruncateAfterBailouts; + return true; // Set truncated flag if range analysis ensure that it has no // rounding errors and no fractional part. Note that we can't use @@ -2433,10 +2425,10 @@ ComputeTruncateKind(MInstruction *candidate) canHaveRoundingErrors = false; if (canHaveRoundingErrors) - return MDefinition::NoTruncate; + return false; // Ensure all observable uses are truncated. - return ComputeRequestedTruncateKind(candidate); + return AllUsesTruncate(candidate); } static void @@ -2463,7 +2455,7 @@ AdjustTruncatedInputs(TempAllocator &alloc, MInstruction *truncated) { MBasicBlock *block = truncated->block(); for (size_t i = 0, e = truncated->numOperands(); i < e; i++) { - if (truncated->operandTruncateKind(i) == MDefinition::NoTruncate) + if (!truncated->isOperandTruncated(i)) continue; MDefinition *input = truncated->getOperand(i); @@ -2523,12 +2515,11 @@ RangeAnalysis::truncate() default:; } - MDefinition::TruncateKind kind = ComputeTruncateKind(*iter); - if (kind == MDefinition::NoTruncate) + if (!CanTruncate(*iter)) continue; // Truncate this instruction if possible. - if (!iter->truncate(kind)) + if (!iter->truncate()) continue; // Delay updates of inputs/outputs to avoid creating node which