Title: [192345] trunk/Source/_javascript_Core
Revision
192345
Author
fpi...@apple.com
Date
2015-11-11 19:39:00 -0800 (Wed, 11 Nov 2015)

Log 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):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (192344 => 192345)


--- trunk/Source/_javascript_Core/ChangeLog	2015-11-12 03:13:39 UTC (rev 192344)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-11-12 03:39:00 UTC (rev 192345)
@@ -1,3 +1,33 @@
+2015-11-11  Filip Pizlo  <fpi...@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....@apple.com>
 
         Add ability to insert probes in LLVM IR that we generate.

Modified: trunk/Source/_javascript_Core/b3/B3StackmapSpecial.cpp (192344 => 192345)


--- trunk/Source/_javascript_Core/b3/B3StackmapSpecial.cpp	2015-11-12 03:13:39 UTC (rev 192344)
+++ trunk/Source/_javascript_Core/b3/B3StackmapSpecial.cpp	2015-11-12 03:39:00 UTC (rev 192345)
@@ -151,9 +151,17 @@
             ASSERT_NOT_REACHED();
             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.
             ASSERT_NOT_REACHED();

Modified: trunk/Source/_javascript_Core/b3/air/AirCode.cpp (192344 => 192345)


--- trunk/Source/_javascript_Core/b3/air/AirCode.cpp	2015-11-12 03:13:39 UTC (rev 192344)
+++ trunk/Source/_javascript_Core/b3/air/AirCode.cpp	2015-11-12 03:39:00 UTC (rev 192345)
@@ -68,6 +68,7 @@
 Special* Code::addSpecial(std::unique_ptr<Special> special)
 {
     Special* result = special.get();
+    result->m_code = this;
     m_specials.append(WTF::move(special));
     return result;
 }

Modified: trunk/Source/_javascript_Core/b3/air/AirSpecial.cpp (192344 => 192345)


--- trunk/Source/_javascript_Core/b3/air/AirSpecial.cpp	2015-11-12 03:13:39 UTC (rev 192344)
+++ trunk/Source/_javascript_Core/b3/air/AirSpecial.cpp	2015-11-12 03:39:00 UTC (rev 192345)
@@ -37,6 +37,7 @@
 
 Special::Special()
     : m_index(UINT_MAX)
+    , m_code(nullptr)
 {
 }
 

Modified: trunk/Source/_javascript_Core/b3/air/AirSpecial.h (192344 => 192345)


--- trunk/Source/_javascript_Core/b3/air/AirSpecial.h	2015-11-12 03:13:39 UTC (rev 192344)
+++ trunk/Source/_javascript_Core/b3/air/AirSpecial.h	2015-11-12 03:39:00 UTC (rev 192345)
@@ -36,6 +36,7 @@
 
 namespace JSC { namespace B3 { namespace Air {
 
+class Code;
 struct GenerationContext;
 
 class Special {
@@ -47,6 +48,8 @@
     Special();
     virtual ~Special();
 
+    Code& code() const { return *m_code; }
+
     CString name() const;
 
     virtual void forEachArg(Inst&, const ScopedLambda<Inst::EachArgCallback>&) = 0;
@@ -97,6 +100,7 @@
 
     const char* m_name;
     unsigned m_index;
+    Code* m_code;
 };
 
 class DeepSpecialDump {

Modified: trunk/Source/_javascript_Core/b3/testb3.cpp (192344 => 192345)


--- trunk/Source/_javascript_Core/b3/testb3.cpp	2015-11-12 03:13:39 UTC (rev 192344)
+++ trunk/Source/_javascript_Core/b3/testb3.cpp	2015-11-12 03:39:00 UTC (rev 192345)
@@ -2577,6 +2577,33 @@
     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);
+
+    CHECK(compileAndRun<int>(proc, 1, 2) == 3);
+}
+
 void testSimpleCheck()
 {
     Procedure proc;
@@ -3606,6 +3633,7 @@
     RUN(testComplex(4, 384));
 
     RUN(testSimplePatchpoint());
+    RUN(testPatchpointCallArg());
     RUN(testSimpleCheck());
     RUN(testCheckLessThan());
     RUN(testCheckMegaCombo());
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to