DFG should update backwards propogation after fixup.

https://bugs.webkit.org/show_bug.cgi?id=257949
rdar://110661900

Reviewed by Keith Miller.

PureInt means that we cannot observe a difference between this value when
represented as a double or when represented as a UInt32.

Today, PureInt is not a proven property, but rather a speculation guide.
The DFG fixup phase is responsible for inserting speculations and fixing
up edges to ensure that we can prove the properties that we want.

UInt32ToNumber speculates that a value fits in an Int32. DoubleRep takes
an Int32 and stuffs the bits appropriately to turn it into a double.

ValueAdd is expecting a DoubleRep because it has a double argument.

In FixupPhase, we remove UInt32ToNumber because we see that it is PureInt.
If it is actually PureInt, then this is fine. But DoubleRep can observe it
as non-PureInt, and DoubleRep not inserted until well after BackwardsPropogationPhase has run.

We add a separate phase that runs after fixup, and pessimizes these speculation properties.
Then, we are free to use them as proven properties.

Finally, we move any checks in fixup that use these properties to strength reduction.

* JSTests/stress/propogate-PureInt-double-use.js: Added.
(opt):
(noInline.opt.o):
(noInline.o.main):
* Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp:
(JSC::DFG::BackwardsPropagationPhase::BackwardsPropagationPhase):
(JSC::DFG::BackwardsPropagationPhase::propagate):
(JSC::DFG::performBackwardsPropagation):
(JSC::DFG::performBackwardsPropagationAfterFixup):
* Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.h:
* Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* Source/JavaScriptCore/dfg/DFGPlan.cpp:
(JSC::DFG::Plan::compileInThreadImpl):
* Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileUInt32ToNumber):
* Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:
(JSC::DFG::StrengthReductionPhase::handleNode):

fix

.

Canonical link: https://commits.webkit.org/265833@main

JSTests/stress/propogate-PureInt-double-use.js

@@ -0,0 +1,24 @@
+function opt(v)
+{
+    let a = (-1 >>> v)
+    let b = (a + -0.2)
+    let c = b | 0
+    return c
+}
+noInline(opt)
+
+function o(v) {
+    opt(v)
+}
+noInline(o)
+
+function main() {
+    for (let i = 0; i < 10000; i++)
+        o(0)
+    for (let i = 0; i < 10000; ++i)
+     opt(32)
+    if (opt(32) != -2)
+        throw "wrong value"
+}
+noInline(main)
+main()

Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp

@@ -41,8 +41,9 @@ namespace JSC { namespace DFG {
 
 class BackwardsPropagationPhase {
 public:
-    BackwardsPropagationPhase(Graph& graph)
+    BackwardsPropagationPhase(Graph& graph, bool fixupHasRun)
         : m_graph(graph)
+        , m_fixupHasRun(fixupHasRun)
         , m_flagsAtHead(graph)
     {
     }
@@ -296,12 +297,12 @@ class BackwardsPropagationPhase {
             mergeFlags(m_currentFlags.operand(variableAccessData->operand()), VariableIsUsed);
             break;
         }
-            
+
         case MovHint:
         case Check:
         case CheckVarargs:
             break;
-            
+
         case ValueBitNot:
         case ArithBitNot: {
             flags |= NodeBytecodeUsesAsInt;
@@ -572,32 +573,54 @@ class BackwardsPropagationPhase {
         }
 
         case Identity: 
-            // This would be trivial to handle but we just assert that we cannot see these yet.
-            RELEASE_ASSERT_NOT_REACHED();
+            RELEASE_ASSERT(m_fixupHasRun);
+            node->child1()->mergeFlags(flags);
             break;
-
+
+        case ValueRep:
+        case DoubleRep:
+        case Int52Rep: {
+            // Since fixup has already run, we are not asking "how should I sepculate," but rather
+            // precisely how this particular value is going to be used.
+            if (m_fixupHasRun && isDouble(node->child1().useKind())) {
+                node->child1()->mergeFlags(NodeBytecodeUsesAsNumber);
+                break;
+            }
+            if (m_fixupHasRun && node->child1().useKind() == Int32Use)
+                break;
+            FALLTHROUGH;
+        }
+
         // Note: ArithSqrt, ArithUnary and other math intrinsics don't have special
         // rules in here because they are always followed by Phantoms to signify that if the
         // method call speculation fails, the bytecode may use the arguments in arbitrary ways.
         // This corresponds to that possibility of someone doing something like:
         // Math.sin = function(x) { doArbitraryThingsTo(x); }
-            
+
         default:
             mergeDefaultFlags(node);
             break;
         }
     }
-    
+
     Graph& m_graph;
     bool m_allowNestedOverflowingAdditions;
+    bool m_fixupHasRun;
 
     BlockMap<Operands<NodeFlags>> m_flagsAtHead;
     Operands<NodeFlags> m_currentFlags;
 };
 
 void performBackwardsPropagation(Graph& graph)
 {
-    BackwardsPropagationPhase(graph).run();
+    constexpr bool fixupHasRun = false;
+    BackwardsPropagationPhase(graph, fixupHasRun).run();
+}
+
+bool performBackwardsPropagationAfterFixup(Graph& graph)
+{
+    constexpr bool fixupHasRun = true;
+    return BackwardsPropagationPhase(graph, fixupHasRun).run();
 }
 
 } } // namespace JSC::DFG

Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.h

@@ -32,11 +32,20 @@ namespace JSC { namespace DFG {
 
 class Graph;
 
-// Infer basic information about how nodes are used by doing a block-local
+// Infer basic information about how nodes are likely to be used by doing a block-local
 // backwards flow analysis.
 
 void performBackwardsPropagation(Graph&);
 
+// Infer information after fixup has run. This should only pessimize the existing information.
+// By this point, we ensure that any new uses inserted by fixup are accounted for.
+//
+// For example, consider:
+// b = (a + 0.1)
+// If b is PureInt, then we would propogate that to a originally because that is what we expect.
+// But we don't actually know until after fixup if a may be used as a double, as it is here.
+bool performBackwardsPropagationAfterFixup(Graph&);
+
 } } // namespace JSC::DFG
 
 #endif // ENABLE(DFG_JIT)

Source/JavaScriptCore/dfg/DFGFixupPhase.cpp

@@ -442,9 +442,7 @@ class FixupPhase : public Phase {
 
         case UInt32ToNumber: {
             fixIntConvertingEdge(node->child1());
-            if (bytecodeCanTruncateInteger(node->arithNodeFlags()))
-                node->convertToIdentity();
-            else if (node->canSpeculateInt32(FixupPass))
+            if (bytecodeCanTruncateInteger(node->arithNodeFlags()) || node->canSpeculateInt32(FixupPass))
                 node->setArithMode(Arith::CheckOverflow);
             else {
                 node->setArithMode(Arith::DoOverflow);

Source/JavaScriptCore/dfg/DFGPlan.cpp

@@ -29,6 +29,7 @@
 #if ENABLE(DFG_JIT)
 
 #include "DFGArgumentsEliminationPhase.h"
+#include "DFGBackwardsPropagationPhase.h"
 #include "DFGByteCodeParser.h"
 #include "DFGCFAPhase.h"
 #include "DFGCFGSimplificationPhase.h"
@@ -270,6 +271,7 @@ Plan::CompilationPath Plan::compileInThreadImpl()
     if (validationEnabled())
         validate(dfg);
 
+    RUN_PHASE(performBackwardsPropagationAfterFixup);
     RUN_PHASE(performStrengthReduction);
     RUN_PHASE(performCPSRethreading);
     RUN_PHASE(performCFA);

Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

@@ -3544,7 +3544,6 @@ void SpeculativeJIT::compileUInt32ToNumber(Node* node)
     GPRTemporary result(this);
 
     move(op1.gpr(), result.gpr());
-
     speculationCheck(ExitKind::Overflow, JSValueRegs(), nullptr, Base::branch32(LessThan, result.gpr(), TrustedImm32(0)));
 
     strictInt32Result(result.gpr(), node, op1.format());

Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp

@@ -114,6 +114,10 @@ class StrengthReductionPhase : public Phase {
                 m_changed = true;
                 break;
             }
+            if (bytecodeCanTruncateInteger(m_node->arithNodeFlags())) {
+                m_node->convertToIdentity();
+                m_changed = true;
+            }
             break;
 
         case ArithAdd: