Changeset 192345 in webkit

Timestamp:

Nov 11, 2015 7:39:00 PM (8 years ago)

Author:

fpizlo@apple.com

Message:

Patchpoints with stackArgument constraints should work
​https://bugs.webkit.org/show_bug.cgi?id=151177

Reviewed by Saam Barati.

The only thing broken was that StackmapSpecial's isValidForm would reject Arg::addr, so
validation would fail after allocateStack.

In order for StackmapSpecial to validate Arg::addr, it needs to know the frame size. To know
the frame size, it needs access to Code&. So, this changes Air::Special to always have a
pointer back to Code.

Other than this already worked.

• b3/B3StackmapSpecial.cpp:

(JSC::B3::StackmapSpecial::isValidImpl):

• b3/air/AirCode.cpp:

(JSC::B3::Air::Code::addSpecial):

• b3/air/AirSpecial.cpp:

(JSC::B3::Air::Special::Special):

• b3/air/AirSpecial.h:

(JSC::B3::Air::Special::code):

• b3/testb3.cpp:

(JSC::B3::testSimplePatchpoint):
(JSC::B3::testPatchpointCallArg):
(JSC::B3::testSimpleCheck):
(JSC::B3::run):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• b3/B3StackmapSpecial.cpp (modified) (1 diff)
• b3/air/AirCode.cpp (modified) (1 diff)
• b3/air/AirSpecial.cpp (modified) (1 diff)
• b3/air/AirSpecial.h (modified) (3 diffs)
• b3/testb3.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2015-11-11  Filip Pizlo  <fpizlo@apple.com>
+
+        Patchpoints with stackArgument constraints should work
+        https://bugs.webkit.org/show_bug.cgi?id=151177
+
+        Reviewed by Saam Barati.
+
+        The only thing broken was that StackmapSpecial's isValidForm would reject Arg::addr, so
+        validation would fail after allocateStack.
+
+        In order for StackmapSpecial to validate Arg::addr, it needs to know the frame size. To know
+        the frame size, it needs access to Code&. So, this changes Air::Special to always have a
+        pointer back to Code.
+
+        Other than this already worked.
+
+        * b3/B3StackmapSpecial.cpp:
+        (JSC::B3::StackmapSpecial::isValidImpl):
+        * b3/air/AirCode.cpp:
+        (JSC::B3::Air::Code::addSpecial):
+        * b3/air/AirSpecial.cpp:
+        (JSC::B3::Air::Special::Special):
+        * b3/air/AirSpecial.h:
+        (JSC::B3::Air::Special::code):
+        * b3/testb3.cpp:
+        (JSC::B3::testSimplePatchpoint):
+        (JSC::B3::testPatchpointCallArg):
+        (JSC::B3::testSimpleCheck):
+        (JSC::B3::run):
+
 2015-11-11  Mark Lam  <mark.lam@apple.com>
 

trunk/Source/JavaScriptCore/b3/B3StackmapSpecial.cpp

@@ -152 +152 @@
             break;
         case ValueRep::StackArgument:
-            if (arg != Arg::callArg(rep.offsetFromSP()))
-                return false;
-            break;
+            if (arg == Arg::callArg(rep.offsetFromSP()))
+                break;
+            if (arg.isAddr() && code().frameSize()) {
+                if (arg.base() == Tmp(GPRInfo::callFrameRegister)
+                    && arg.offset() == rep.offsetFromSP() - code().frameSize())
+                    break;
+                if (arg.base() == Tmp(MacroAssembler::stackPointerRegister)
+                    && arg.offset() == rep.offsetFromSP())
+                    break;
+            }
+            return false;
         case ValueRep::Constant:
             // This is not a valid input representation.

trunk/Source/JavaScriptCore/b3/air/AirCode.cpp

@@ -69 +69 @@
 {
     Special* result = special.get();
+    result->m_code = this;
     m_specials.append(WTF::move(special));
     return result;

trunk/Source/JavaScriptCore/b3/air/AirSpecial.cpp

@@ -38 +38 @@
 Special::Special()
     : m_index(UINT_MAX)
+    , m_code(nullptr)
 {
 }

trunk/Source/JavaScriptCore/b3/air/AirSpecial.h

@@ -37 +37 @@
 namespace JSC { namespace B3 { namespace Air {
 
+class Code;
 struct GenerationContext;
 
@@ -47 +48 @@
     Special();
     virtual ~Special();
+
+    Code& code() const { return *m_code; }
 
     CString name() const;
@@ -98 +101 @@
     const char* m_name;
     unsigned m_index;
+    Code* m_code;
 };
 

trunk/Source/JavaScriptCore/b3/testb3.cpp

@@ -2572 +2572 @@
             jit.move(params.reps[1].gpr(), params.reps[0].gpr());
             jit.add32(params.reps[2].gpr(), params.reps[0].gpr());
+        });
+    root->appendNew<ControlValue>(proc, Return, Origin(), patchpoint);
+
+    CHECK(compileAndRun<int>(proc, 1, 2) == 3);
+}
+
+void testPatchpointCallArg()
+{
+    Procedure proc;
+    BasicBlock* root = proc.addBlock();
+    Value* arg1 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0);
+    Value* arg2 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1);
+    PatchpointValue* patchpoint = root->appendNew<PatchpointValue>(proc, Int32, Origin());
+    patchpoint->append(ConstrainedValue(arg1, ValueRep::stackArgument(0)));
+    patchpoint->append(ConstrainedValue(arg2, ValueRep::stackArgument(8)));
+    patchpoint->setGenerator(
+        [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
+            CHECK(params.reps.size() == 3);
+            CHECK(params.reps[0].isGPR());
+            CHECK(params.reps[1].isStack());
+            CHECK(params.reps[2].isStack());
+            jit.load32(
+                CCallHelpers::Address(GPRInfo::callFrameRegister, params.reps[1].offsetFromFP()),
+                params.reps[0].gpr());
+            jit.add32(
+                CCallHelpers::Address(GPRInfo::callFrameRegister, params.reps[2].offsetFromFP()),
+                params.reps[0].gpr());
         });
     root->appendNew<ControlValue>(proc, Return, Origin(), patchpoint);
@@ -3607 +3634 @@
 
     RUN(testSimplePatchpoint());
+    RUN(testPatchpointCallArg());
     RUN(testSimpleCheck());
     RUN(testCheckLessThan());