lowerStackArgs should lower Lea32/64 on ARM64 to Add
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Feb 2019 06:37:50 +0000 (06:37 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Feb 2019 06:37:50 +0000 (06:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=194656

Reviewed by Yusuke Suzuki.

On arm64, Lea is just implemented as an add. However, Air treats it as an
address with a given width. Because of this width, we were incorrectly
computing whether or not this immediate could fit into the instruction itself
or it needed to be explicitly put into a register. This patch makes
AirLowerStackArgs lower Lea to Add on arm64.

* b3/air/AirLowerStackArgs.cpp:
(JSC::B3::Air::lowerStackArgs):
* b3/air/AirOpcode.opcodes:
* b3/air/testair.cpp:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241577 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/b3/air/AirLowerStackArgs.cpp
Source/JavaScriptCore/b3/air/AirOpcode.opcodes
Source/JavaScriptCore/b3/air/testair.cpp

index 0bf3b11..24db8ff 100644 (file)
@@ -1,3 +1,21 @@
+2019-02-14  Saam barati  <sbarati@apple.com>
+
+        lowerStackArgs should lower Lea32/64 on ARM64 to Add
+        https://bugs.webkit.org/show_bug.cgi?id=194656
+
+        Reviewed by Yusuke Suzuki.
+
+        On arm64, Lea is just implemented as an add. However, Air treats it as an
+        address with a given width. Because of this width, we were incorrectly
+        computing whether or not this immediate could fit into the instruction itself
+        or it needed to be explicitly put into a register. This patch makes
+        AirLowerStackArgs lower Lea to Add on arm64.
+
+        * b3/air/AirLowerStackArgs.cpp:
+        (JSC::B3::Air::lowerStackArgs):
+        * b3/air/AirOpcode.opcodes:
+        * b3/air/testair.cpp:
+
 2019-02-14  Saam Barati  <sbarati@apple.com>
 
         Cache the results of BytecodeGenerator::getVariablesUnderTDZ
index f63bd41..23e3fbd 100644 (file)
@@ -71,6 +71,46 @@ void lowerStackArgs(Code& code)
         for (unsigned instIndex = 0; instIndex < block->size(); ++instIndex) {
             Inst& inst = block->at(instIndex);
 
+            if (isARM64() && (inst.kind.opcode == Lea32 || inst.kind.opcode == Lea64)) {
+                // On ARM64, Lea is just an add. We can't handle this below because
+                // taking into account the Width to see if we can compute the immediate
+                // is wrong.
+                auto lowerArmLea = [&] (Value::OffsetType offset, Tmp base) {
+                    ASSERT(inst.args[1].isTmp());
+
+                    if (Arg::isValidImmForm(offset))
+                        inst = Inst(inst.kind.opcode == Lea32 ? Add32 : Add64, inst.origin, Arg::imm(offset), base, inst.args[1]);
+                    else {
+                        ASSERT(pinnedExtendedOffsetAddrRegister());
+                        Air::Tmp tmp = Air::Tmp(*pinnedExtendedOffsetAddrRegister());
+                        Arg offsetArg = Arg::bigImm(offset);
+                        insertionSet.insert(instIndex, Move, inst.origin, offsetArg, tmp);
+                        inst = Inst(inst.kind.opcode == Lea32 ? Add32 : Add64, inst.origin, tmp, base, inst.args[1]);
+                    }
+                };
+
+                switch (inst.args[0].kind()) {
+                case Arg::Stack: {
+                    StackSlot* slot = inst.args[0].stackSlot();
+                    lowerArmLea(inst.args[0].offset() + slot->offsetFromFP(), Tmp(GPRInfo::callFrameRegister));
+                    break;
+                }
+                case Arg::CallArg:
+                    lowerArmLea(inst.args[0].offset() - code.frameSize(), Tmp(GPRInfo::callFrameRegister));
+                    break;
+                case Arg::Addr:
+                    lowerArmLea(inst.args[0].offset(), inst.args[0].base());
+                    break;
+                case Arg::ExtendedOffsetAddr:
+                    ASSERT_NOT_REACHED();
+                    break;
+                default:
+                    break;
+                }
+
+                continue;
+            }
+
             inst.forEachArg(
                 [&] (Arg& arg, Arg::Role role, Bank, Width width) {
                     auto stackAddr = [&] (Value::OffsetType offsetFromFP) -> Arg {
index 873af69..11ada41 100644 (file)
@@ -325,6 +325,9 @@ x86_64: X86Div64 UZD:G:64, UZD:G:64, U:G:64
 x86_64: X86UDiv64 UZD:G:64, UZD:G:64, U:G:64
     Tmp*, Tmp*, Tmp
 
+# If we add other things like Lea that are UA, we may need to lower
+# them on arm64 similarly to how we do for Lea. In lowerStackArgs,
+# we lower Lea to add on arm64.
 Lea32 UA:G:32, D:G:32
     Addr, Tmp
     x86: Index, Tmp as x86Lea32
index 912194f..498487e 100644 (file)
@@ -2028,6 +2028,40 @@ void testArgumentRegPinned3()
     CHECK(r == 10 + 42 + 42);
 }
 
+void testLea64()
+{
+    B3::Procedure proc;
+    Code& code = proc.code();
+
+    BasicBlock* root = code.addBlock();
+
+    int64_t a = 0x11223344;
+    int64_t b = 1 << 13;
+
+    root->append(Lea64, nullptr, Arg::addr(Tmp(GPRInfo::argumentGPR0), b), Tmp(GPRInfo::returnValueGPR));
+    root->append(Ret64, nullptr, Tmp(GPRInfo::returnValueGPR));
+
+    int64_t r = compileAndRun<int64_t>(proc, a);
+    CHECK(r == a + b);
+}
+
+void testLea32()
+{
+    B3::Procedure proc;
+    Code& code = proc.code();
+
+    BasicBlock* root = code.addBlock();
+
+    int32_t a = 0x11223344;
+    int32_t b = 1 << 13;
+
+    root->append(Lea32, nullptr, Arg::addr(Tmp(GPRInfo::argumentGPR0), b), Tmp(GPRInfo::returnValueGPR));
+    root->append(Ret32, nullptr, Tmp(GPRInfo::returnValueGPR));
+
+    int32_t r = compileAndRun<int32_t>(proc, a);
+    CHECK(r == a + b);
+}
+
 #define RUN(test) do {                          \
         if (!shouldRun(#test))                  \
             break;                              \
@@ -2113,6 +2147,9 @@ void run(const char* filter)
     RUN(testArgumentRegPinned2());
     RUN(testArgumentRegPinned3());
 
+    RUN(testLea32());
+    RUN(testLea64());
+
     if (tasks.isEmpty())
         usage();