Title: [255538] trunk/Source/_javascript_Core
Revision
255538
Author
sbar...@apple.com
Date
2020-01-31 17:47:42 -0800 (Fri, 31 Jan 2020)

Log Message

safe to execute should return false when we know code won't be moved
https://bugs.webkit.org/show_bug.cgi?id=207074

Reviewed by Yusuke Suzuki.

We use safeToExecute to determine inside LICM whether it's safe to execute
a node somewhere else in the program. We were returning true for nodes
we knew would never be moved, because they were effectful. Things like Call
and GetById. This patch makes those nodes return false now, since we want
to make it easier to audit the nodes that return true. This makes that audit
easier, since it gets rid of the obvious things that will never be hoisted.

* dfg/DFGSafeToExecute.h:
(JSC::DFG::safeToExecute):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (255537 => 255538)


--- trunk/Source/_javascript_Core/ChangeLog	2020-02-01 01:22:31 UTC (rev 255537)
+++ trunk/Source/_javascript_Core/ChangeLog	2020-02-01 01:47:42 UTC (rev 255538)
@@ -1,5 +1,22 @@
 2020-01-31  Saam Barati  <sbar...@apple.com>
 
+        safe to execute should return false when we know code won't be moved
+        https://bugs.webkit.org/show_bug.cgi?id=207074
+
+        Reviewed by Yusuke Suzuki.
+
+        We use safeToExecute to determine inside LICM whether it's safe to execute
+        a node somewhere else in the program. We were returning true for nodes
+        we knew would never be moved, because they were effectful. Things like Call
+        and GetById. This patch makes those nodes return false now, since we want
+        to make it easier to audit the nodes that return true. This makes that audit
+        easier, since it gets rid of the obvious things that will never be hoisted.
+
+        * dfg/DFGSafeToExecute.h:
+        (JSC::DFG::safeToExecute):
+
+2020-01-31  Saam Barati  <sbar...@apple.com>
+
         GetGetterSetterByOffset and GetGetter/GetSetter are not always safe to execute
         https://bugs.webkit.org/show_bug.cgi?id=206805
         <rdar://problem/58898161>

Modified: trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h (255537 => 255538)


--- trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h	2020-02-01 01:22:31 UTC (rev 255537)
+++ trunk/Source/_javascript_Core/dfg/DFGSafeToExecute.h	2020-02-01 01:47:42 UTC (rev 255538)
@@ -174,10 +174,14 @@
         }
     }
 
-    // NOTE: This tends to lie when it comes to effectful nodes, because it knows that they aren't going to
-    // get hoisted anyway.
+    // NOTE: This can lie when it comes to effectful nodes, because it knows that they aren't going to
+    // get hoisted anyway. Sometimes this is convenient so we can avoid branching on some internal
+    // state of the node (like what some child's UseKind might be). However, nodes that are obviously
+    // always effectful, we return false for, to make auditing the "return true" cases easier.
 
     switch (node->op()) {
+    // FIXME: Audit these:
+    // https://bugs.webkit.org/show_bug.cgi?id=207075
     case JSConstant:
     case DoubleConstant:
     case Int52Constant:
@@ -184,33 +188,13 @@
     case LazyJSConstant:
     case Identity:
     case IdentityWithProfile:
-    case ToThis:
-    case CreateThis:
-    case CreatePromise:
-    case CreateGenerator:
-    case CreateAsyncGenerator:
-    case ObjectCreate:
-    case ObjectKeys:
     case GetCallee:
-    case SetCallee:
     case GetArgumentCountIncludingThis:
-    case SetArgumentCountIncludingThis:
     case GetRestLength:
     case GetLocal:
-    case SetLocal:
-    case PutStack:
-    case KillStack:
     case GetStack:
-    case MovHint:
-    case ZombieHint:
     case ExitOK:
     case Phantom:
-    case Upsilon:
-    case Phi:
-    case Flush:
-    case PhantomLocal:
-    case SetArgumentDefinitely:
-    case SetArgumentMaybe:
     case ArithBitNot:
     case ArithBitAnd:
     case ArithBitOr:
@@ -241,42 +225,7 @@
     case ArithCeil:
     case ArithTrunc:
     case ArithUnary:
-    case ValueBitAnd:
-    case ValueBitXor:
-    case ValueBitOr:
-    case ValueBitNot:
-    case ValueBitLShift:
-    case ValueBitRShift:
-    case Inc:
-    case Dec:
-    case ValueNegate:
-    case ValueAdd:
-    case ValueSub:
-    case ValueMul:
-    case ValueDiv:
-    case ValueMod:
-    case ValuePow:
-    case TryGetById:
-    case DeleteById:
-    case DeleteByVal:
-    case GetById:
-    case GetByIdWithThis:
-    case GetByValWithThis:
-    case GetByIdFlush:
-    case GetByIdDirect:
-    case GetByIdDirectFlush:
-    case PutById:
-    case PutByIdFlush:
-    case PutByIdWithThis:
-    case PutByValWithThis:
-    case PutByIdDirect:
-    case PutGetterById:
-    case PutSetterById:
-    case PutGetterSetterById:
-    case PutGetterByVal:
-    case PutSetterByVal:
-    case DefineDataProperty:
-    case DefineAccessorProperty:
+    case TryGetById: // FIXME: Audit this: https://bugs.webkit.org/show_bug.cgi?id=163834
     case CheckStructure:
     case CheckStructureOrEmpty:
     case GetExecutable:
@@ -286,27 +235,17 @@
     case CheckSubClass:
     case CheckArray:
     case CheckArrayOrEmpty:
-    case Arrayify:
-    case ArrayifyToStructure:
     case GetScope:
     case SkipScope:
     case GetGlobalObject:
     case GetGlobalThis:
     case GetClosureVar:
-    case PutClosureVar:
     case GetGlobalVar:
     case GetGlobalLexicalVariable:
-    case PutGlobalVariable:
     case CheckCell:
-    case CheckBadCell:
     case CheckNotEmpty:
     case AssertNotEmpty:
     case CheckIdent:
-    case RegExpExec:
-    case RegExpExecNonGlobalOrSticky:
-    case RegExpTest:
-    case RegExpMatchFast:
-    case RegExpMatchFastGlobal:
     case CompareLess:
     case CompareLessEq:
     case CompareGreater:
@@ -317,40 +256,9 @@
     case CompareStrictEq:
     case CompareEqPtr:
     case SameValue:
-    case Call:
-    case DirectCall:
-    case TailCallInlinedCaller:
-    case DirectTailCallInlinedCaller:
-    case Construct:
-    case DirectConstruct:
-    case CallVarargs:
-    case CallEval:
-    case TailCallVarargsInlinedCaller:
-    case TailCallForwardVarargsInlinedCaller:
-    case ConstructVarargs:
-    case VarargsLength:
-    case LoadVarargs:
-    case CallForwardVarargs:
-    case ConstructForwardVarargs:
-    case NewObject:
-    case NewPromise:
-    case NewGenerator:
-    case NewAsyncGenerator:
-    case NewArray:
-    case NewArrayWithSize:
-    case NewArrayBuffer:
-    case NewArrayWithSpread:
-    case NewArrayIterator:
-    case Spread:
-    case NewRegexp:
-    case NewSymbol:
-    case ProfileType:
-    case ProfileControlFlow:
     case CheckTypeInfoFlags:
     case ParseInt:
     case OverridesHasInstance:
-    case InstanceOf:
-    case InstanceOfCustom:
     case IsEmpty:
     case IsUndefined:
     case IsUndefinedOrNull:
@@ -364,122 +272,38 @@
     case IsTypedArrayView:
     case TypeOf:
     case LogicalNot:
-    case CallObjectConstructor:
-    case ToPrimitive:
-    case ToPropertyKey:
     case ToString:
-    case ToNumber:
-    case ToNumeric:
-    case ToObject:
-    case NumberToStringWithRadix:
     case NumberToStringWithValidRadixConstant:
-    case SetFunctionName:
     case StrCat:
     case CallStringConstructor:
-    case NewStringObject:
     case MakeRope:
-    case InByVal:
-    case InById:
-    case HasOwnProperty:
-    case PushWithScope:
-    case CreateActivation:
-    case CreateDirectArguments:
-    case CreateScopedArguments:
-    case CreateClonedArguments:
-    case CreateArgumentsButterfly:
     case GetFromArguments:
     case GetArgument:
-    case PutToArguments:
-    case NewFunction:
-    case NewGeneratorFunction:
-    case NewAsyncGeneratorFunction:
-    case NewAsyncFunction:
-    case Jump:
-    case Branch:
-    case Switch:
-    case EntrySwitch:
-    case Return:
-    case TailCall:
-    case DirectTailCall:
-    case TailCallVarargs:
-    case TailCallForwardVarargs:
-    case Throw:
-    case ThrowStaticError:
-    case CountExecution:
-    case SuperSamplerBegin:
-    case SuperSamplerEnd:
-    case ForceOSRExit:
-    case CPUIntrinsic:
-    case CheckTraps:
-    case LogShadowChickenPrologue:
-    case LogShadowChickenTail:
     case StringFromCharCode:
-    case NewTypedArray:
-    case Unreachable:
     case ExtractOSREntryLocal:
     case ExtractCatchLocal:
-    case ClearCatchLocals:
-    case CheckTierUpInLoop:
-    case CheckTierUpAtReturn:
-    case CheckTierUpAndOSREnter:
-    case LoopHint:
-    case InvalidationPoint:
-    case NotifyWrite:
     case CheckInBounds:
     case ConstantStoragePointer:
     case Check:
     case CheckVarargs:
-    case MultiPutByOffset:
     case ValueRep:
     case DoubleRep:
     case Int52Rep:
     case BooleanToNumber:
     case FiatInt52:
-    case GetEnumerableLength:
-    case HasGenericProperty:
-    case HasStructureProperty:
     case HasIndexedProperty:
-    case GetDirectPname:
-    case GetPropertyEnumerator:
     case GetEnumeratorStructurePname:
     case GetEnumeratorGenericPname:
     case ToIndexString:
-    case PhantomNewObject:
-    case PhantomNewFunction:
-    case PhantomNewGeneratorFunction:
-    case PhantomNewAsyncGeneratorFunction:
-    case PhantomNewAsyncFunction:
-    case PhantomNewArrayIterator:
-    case PhantomCreateActivation:
-    case PhantomNewRegexp:
-    case PutHint:
     case CheckStructureImmediate:
-    case MaterializeNewObject:
-    case MaterializeCreateActivation:
-    case MaterializeNewInternalFieldObject:
-    case PhantomDirectArguments:
-    case PhantomCreateRest:
-    case PhantomSpread:
-    case PhantomNewArrayWithSpread:
-    case PhantomNewArrayBuffer:
-    case PhantomClonedArguments:
     case GetMyArgumentByVal:
     case GetMyArgumentByValOutOfBounds:
-    case ForwardVarargs:
-    case CreateRest:
     case GetPrototypeOf:
     case StringReplace:
     case StringReplaceRegExp:
     case GetRegExpObjectLastIndex:
-    case SetRegExpObjectLastIndex:
-    case RecordRegExpCachedResult:
-    case GetDynamicVar:
-    case PutDynamicVar:
-    case ResolveScopeForHoistingFuncDeclInEval:
-    case ResolveScope:
     case MapHash:
     case NormalizeMapKey:
-    case StringValueOf:
     case StringSlice:
     case ToLowerCase:
     case GetMapBucket:
@@ -489,19 +313,7 @@
     case LoadValueFromMapBucket:
     case ExtractValueFromWeakMapGet:
     case WeakMapGet:
-    case WeakSetAdd:
-    case WeakMapSet:
-    case AtomicsAdd:
-    case AtomicsAnd:
-    case AtomicsCompareExchange:
-    case AtomicsExchange:
-    case AtomicsLoad:
-    case AtomicsOr:
-    case AtomicsStore:
-    case AtomicsSub:
-    case AtomicsXor:
     case AtomicsIsLockFree:
-    case InitializeEntrypointArguments:
     case MatchStructure:
     case DateGetInt32OrNaN:
     case DateGetTime:
@@ -661,17 +473,211 @@
         return true;
     }
 
+    case ToThis:
+    case CreateThis:
+    case CreatePromise:
+    case CreateGenerator:
+    case CreateAsyncGenerator:
+    case ObjectCreate:
+    case ObjectKeys:
+    case SetLocal:
+    case SetCallee:
+    case PutStack:
+    case KillStack:
+    case MovHint:
+    case ZombieHint:
+    case Upsilon:
+    case Phi:
+    case Flush:
+    case SetArgumentDefinitely:
+    case SetArgumentMaybe:
+    case SetArgumentCountIncludingThis:
+    case PhantomLocal:
+    case DeleteById:
+    case DeleteByVal:
+    case GetById:
+    case GetByIdWithThis:
+    case GetByValWithThis:
+    case GetByIdFlush:
+    case GetByIdDirect:
+    case GetByIdDirectFlush:
+    case PutById:
+    case PutByIdFlush:
+    case PutByIdWithThis:
+    case PutByValWithThis:
+    case PutByIdDirect:
+    case PutGetterById:
+    case PutSetterById:
+    case PutGetterSetterById:
+    case PutGetterByVal:
+    case PutSetterByVal:
+    case DefineDataProperty:
+    case DefineAccessorProperty:
+    case Arrayify:
+    case ArrayifyToStructure:
+    case PutClosureVar:
+    case PutGlobalVariable:
+    case CheckBadCell:
+    case RegExpExec:
+    case RegExpExecNonGlobalOrSticky:
+    case RegExpTest:
+    case RegExpMatchFast:
+    case RegExpMatchFastGlobal:
+    case Call:
+    case DirectCall:
+    case TailCallInlinedCaller:
+    case DirectTailCallInlinedCaller:
+    case Construct:
+    case DirectConstruct:
+    case CallVarargs:
+    case CallEval:
+    case TailCallVarargsInlinedCaller:
+    case TailCallForwardVarargsInlinedCaller:
+    case ConstructVarargs:
+    case VarargsLength:
+    case LoadVarargs:
+    case CallForwardVarargs:
+    case ConstructForwardVarargs:
+    case NewObject:
+    case NewPromise:
+    case NewGenerator:
+    case NewAsyncGenerator:
+    case NewArray:
+    case NewArrayWithSize:
+    case NewArrayBuffer:
+    case NewArrayWithSpread:
+    case NewArrayIterator:
+    case Spread:
+    case NewRegexp:
+    case NewSymbol:
+    case ProfileType:
+    case ProfileControlFlow:
+    case InstanceOf:
+    case InstanceOfCustom:
+    case CallObjectConstructor:
+    case ToPrimitive:
+    case ToPropertyKey:
+    case ToNumber:
+    case ToNumeric:
+    case ToObject:
+    case NumberToStringWithRadix:
+    case SetFunctionName:
+    case NewStringObject:
+    case InByVal:
+    case InById:
+    case HasOwnProperty:
+    case PushWithScope:
+    case CreateActivation:
+    case CreateDirectArguments:
+    case CreateScopedArguments:
+    case CreateClonedArguments:
+    case CreateArgumentsButterfly:
+    case PutToArguments:
+    case NewFunction:
+    case NewGeneratorFunction:
+    case NewAsyncGeneratorFunction:
+    case NewAsyncFunction:
+    case Jump:
+    case Branch:
+    case Switch:
+    case EntrySwitch:
+    case Return:
+    case TailCall:
+    case DirectTailCall:
+    case TailCallVarargs:
+    case TailCallForwardVarargs:
+    case Throw:
+    case ThrowStaticError:
+    case CountExecution:
+    case SuperSamplerBegin:
+    case SuperSamplerEnd:
+    case ForceOSRExit:
+    case CPUIntrinsic:
+    case CheckTraps:
+    case LogShadowChickenPrologue:
+    case LogShadowChickenTail:
+    case NewTypedArray:
+    case Unreachable:
+    case ClearCatchLocals:
+    case CheckTierUpInLoop:
+    case CheckTierUpAtReturn:
+    case CheckTierUpAndOSREnter:
+    case LoopHint:
+    case InvalidationPoint:
+    case NotifyWrite:
+    case MultiPutByOffset:
+    case GetEnumerableLength:
+    case HasGenericProperty:
+    case HasStructureProperty:
+    case GetDirectPname:
+    case GetPropertyEnumerator:
+    case PhantomNewObject:
+    case PhantomNewFunction:
+    case PhantomNewGeneratorFunction:
+    case PhantomNewAsyncGeneratorFunction:
+    case PhantomNewAsyncFunction:
+    case PhantomNewArrayIterator:
+    case PhantomCreateActivation:
+    case PhantomNewRegexp:
+    case PutHint:
+    case MaterializeNewObject:
+    case MaterializeCreateActivation:
+    case MaterializeNewInternalFieldObject:
+    case PhantomDirectArguments:
+    case PhantomCreateRest:
+    case PhantomSpread:
+    case PhantomNewArrayWithSpread:
+    case PhantomNewArrayBuffer:
+    case PhantomClonedArguments:
+    case ForwardVarargs:
+    case CreateRest:
+    case SetRegExpObjectLastIndex:
+    case RecordRegExpCachedResult:
+    case GetDynamicVar:
+    case PutDynamicVar:
+    case ResolveScopeForHoistingFuncDeclInEval:
+    case ResolveScope:
+    case StringValueOf:
+    case WeakSetAdd:
+    case WeakMapSet:
+    case AtomicsAdd:
+    case AtomicsAnd:
+    case AtomicsCompareExchange:
+    case AtomicsExchange:
+    case AtomicsLoad:
+    case AtomicsOr:
+    case AtomicsStore:
+    case AtomicsSub:
+    case AtomicsXor:
+    case InitializeEntrypointArguments:
+    case ValueNegate:
     case GetInternalField:
     case PutInternalField:
-        return false;
-
     case DataViewSet:
-        return false;
-
     case SetAdd:
     case MapSet:
         return false;
 
+    case Inc:
+    case Dec:
+        return node->child1().useKind() != UntypedUse;
+
+    case ValueBitAnd:
+    case ValueBitXor:
+    case ValueBitOr:
+    case ValueBitLShift:
+    case ValueBitRShift:
+    case ValueAdd:
+    case ValueSub:
+    case ValueMul:
+    case ValueDiv:
+    case ValueMod:
+    case ValuePow:
+        return node->isBinaryUseKind(BigIntUse);
+
+    case ValueBitNot:
+        return node->child1().useKind() == BigIntUse;
+
     case LastNodeType:
         RELEASE_ASSERT_NOT_REACHED();
         return false;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to