Changeset 280583 in webkit

Timestamp:

Aug 2, 2021, 9:18:22 PM (5 years ago)

Author:

Yijia Huang

Message:

Add a new pattern to instruction selector to utilize UMULL supported by ARM64
​https://bugs.webkit.org/show_bug.cgi?id=228721

Reviewed by Saam Barati.

Unsigned Multiply Long (UMULL) multiplies two 32-bit register values, and writes the
result to the destination register. This instruction is an alias of the UMADDL instruction.

umull xd wn wm

The equivalent pattern is: d = ZExt32(n) * ZExt32(m)

Given B3 IR:
Int @0 = ArgumentReg(%x0)
Int @1 = Trunc(@0)
Int @2 = ArgumentReg(%x1)
Int @3 = Trunc(@2)
Int @4 = ZExt32(@1)
Int @5 = ZExt32(@3)
Int @6 = Mul(@4, @5)
Void@7 = Return(@6, Terminal)

Old optimized AIR
Move %x0, %x0, @4
Move %x1, %x1, @5
Mul %x0, %x1, %x0, @6
Ret %x0, @7

New optimized AIR
MultiplyZeroExtend %x0, %x1, %x0, @6
Ret %x0, @7

• assembler/MacroAssemblerARM64.h:

(JSC::MacroAssemblerARM64::multiplyZeroExtend32):

• assembler/testmasm.cpp:

(JSC::testMultiplyZeroExtend32):

• b3/B3LowerToAir.cpp:
• b3/air/AirOpcode.opcodes:
• b3/testb3.h:
• b3/testb3_2.cpp:

(testMulArgs32SignExtend):
(testMulArgs32ZeroExtend):

• b3/testb3_3.cpp:

(addArgTests):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• assembler/MacroAssemblerARM64.h (modified) (1 diff)
• assembler/testmasm.cpp (modified) (2 diffs)
• b3/B3LowerToAir.cpp (modified) (11 diffs)
• b3/air/AirOpcode.opcodes (modified) (1 diff)
• b3/testb3.h (modified) (1 diff)
• b3/testb3_2.cpp (modified) (2 diffs)
• b3/testb3_3.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-08-02  Yijia Huang  <yijia_huang@apple.com>
+
+        Add a new pattern to instruction selector to utilize UMULL supported by ARM64
+        https://bugs.webkit.org/show_bug.cgi?id=228721
+
+        Reviewed by Saam Barati.
+
+        Unsigned Multiply Long (UMULL) multiplies two 32-bit register values, and writes the 
+        result to the destination register. This instruction is an alias of the UMADDL instruction.
+
+            umull xd wn wm
+
+        The equivalent pattern is: d = ZExt32(n) * ZExt32(m)
+
+        Given B3 IR:
+        Int @0 = ArgumentReg(%x0)
+        Int @1 = Trunc(@0)
+        Int @2 = ArgumentReg(%x1)
+        Int @3 = Trunc(@2)
+        Int @4 = ZExt32(@1)
+        Int @5 = ZExt32(@3)
+        Int @6 = Mul(@4, @5)
+        Void@7 = Return(@6, Terminal)
+
+        // Old optimized AIR
+        Move   %x0, %x0,      @4
+        Move   %x1, %x1,      @5
+        Mul    %x0, %x1, %x0, @6
+        Ret    %x0,           @7
+
+        // New optimized AIR
+        MultiplyZeroExtend %x0, %x1, %x0, @6
+        Ret                %x0,           @7
+
+        * assembler/MacroAssemblerARM64.h:
+        (JSC::MacroAssemblerARM64::multiplyZeroExtend32):
+        * assembler/testmasm.cpp:
+        (JSC::testMultiplyZeroExtend32):
+        * b3/B3LowerToAir.cpp:
+        * b3/air/AirOpcode.opcodes:
+        * b3/testb3.h:
+        * b3/testb3_2.cpp:
+        (testMulArgs32SignExtend):
+        (testMulArgs32ZeroExtend):
+        * b3/testb3_3.cpp:
+        (addArgTests):
+
 2021-08-02  Yijia Huang  <yijia_huang@apple.com>
 

trunk/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h

@@ -954 +954 @@
     }
 
+    void multiplyZeroExtend32(RegisterID left, RegisterID right, RegisterID dest)
+    {
+        m_assembler.umull(dest, left, right);
+    }
+
     void div32(RegisterID dividend, RegisterID divisor, RegisterID dest)
     {

trunk/Source/JavaScriptCore/assembler/testmasm.cpp

@@ -922 +922 @@
 }
 
+void testMultiplyZeroExtend32()
+{
+    for (auto nOperand : int32Operands()) {
+        auto mul = compile([=] (CCallHelpers& jit) {
+            emitFunctionPrologue(jit);
+
+            jit.multiplyZeroExtend32(GPRInfo::argumentGPR0, GPRInfo::argumentGPR1, GPRInfo::returnValueGPR);
+
+            emitFunctionEpilogue(jit);
+            jit.ret();
+        });
+
+        for (auto mOperand : int32Operands()) {
+            uint32_t n = nOperand;
+            uint32_t m = mOperand;
+            CHECK_EQ(invoke<uint64_t>(mul, n, m), static_cast<uint64_t>(n) * static_cast<uint64_t>(m));
+        }
+    }
+}
+
 void testMultiplyAddSignExtend32()
 {
@@ -5650 +5670 @@
     RUN(testLoadStorePair64Double());
     RUN(testMultiplySignExtend32());
+    RUN(testMultiplyZeroExtend32());
 
     RUN(testSub32Args());

trunk/Source/JavaScriptCore/b3/B3LowerToAir.cpp

@@ -500 +500 @@
     }
 
-    bool isMergeableValue(Value* v, B3::Opcode b3Opcode, bool checkCanBeInternal)
+    bool isMergeableValue(Value* v, B3::Opcode b3Opcode, bool checkCanBeInternal = false)
     { 
         if (v->opcode() != b3Opcode)
@@ -517 +517 @@
         // Maybe, the ideal approach is to introduce a decorator (Index@EXT) to the Air operand 
         // to provide an extension opportunity for the specific form under the Air opcode.
-        if (isMergeableValue(index, ZExt32, false))
+        if (isMergeableValue(index, ZExt32))
             return Arg::index(base, tmp(index->child(0)), scale, offset, MacroAssembler::Extend::ZExt32);
-        if (isMergeableValue(index, SExt32, false))
+        if (isMergeableValue(index, SExt32))
             return Arg::index(base, tmp(index->child(0)), scale, offset, MacroAssembler::Extend::SExt32);
 #endif
@@ -2690 +2690 @@
                             return Air::Oops;
                         // SMADDL: d = SExt32(n) * SExt32(m) + a
-                        if (isMergeableValue(multiplyLeft, SExt32, true) && isMergeableValue(multiplyRight, SExt32, true)) 
+                        if (isMergeableValue(multiplyLeft, SExt32) && isMergeableValue(multiplyRight, SExt32))
                             return MultiplyAddSignExtend32;
                         // UMADDL: d = ZExt32(n) * ZExt32(m) + a
-                        if (isMergeableValue(multiplyLeft, ZExt32, true) && isMergeableValue(multiplyRight, ZExt32, true)) 
+                        if (isMergeableValue(multiplyLeft, ZExt32) && isMergeableValue(multiplyRight, ZExt32))
                             return MultiplyAddZeroExtend32;
                         return Air::Oops;
@@ -2701 +2701 @@
                     if (isValidForm(newAirOpcode, Arg::Tmp, Arg::Tmp, Arg::Tmp, Arg::Tmp)) {
                         append(newAirOpcode, tmp(multiplyLeft->child(0)), tmp(multiplyRight->child(0)), tmp(right), tmp(m_value));
-                        commitInternal(multiplyLeft);
-                        commitInternal(multiplyRight);
                         commitInternal(left);
                         return true;
@@ -2756 +2754 @@
                         return Air::Oops;
                     // SMSUBL: d = a - SExt32(n) * SExt32(m)
-                    if (isMergeableValue(multiplyLeft, SExt32, true) && isMergeableValue(multiplyRight, SExt32, true)) 
+                    if (isMergeableValue(multiplyLeft, SExt32) && isMergeableValue(multiplyRight, SExt32))
                         return MultiplySubSignExtend32;
                     // UMSUBL: d = a - ZExt32(n) * ZExt32(m)
-                    if (isMergeableValue(multiplyLeft, ZExt32, true) && isMergeableValue(multiplyRight, ZExt32, true)) 
+                    if (isMergeableValue(multiplyLeft, ZExt32) && isMergeableValue(multiplyRight, ZExt32))
                         return MultiplySubZeroExtend32;
                     return Air::Oops;
@@ -2767 +2765 @@
                 if (isValidForm(newAirOpcode, Arg::Tmp, Arg::Tmp, Arg::Tmp, Arg::Tmp)) {
                     append(newAirOpcode, tmp(multiplyLeft->child(0)), tmp(multiplyRight->child(0)), tmp(left), tmp(m_value));
-                    commitInternal(multiplyLeft);
-                    commitInternal(multiplyRight);
                     commitInternal(right);
                     return true;
@@ -2809 +2805 @@
                         return Air::Oops;
                     // SMNEGL: d = -(SExt32(n) * SExt32(m))
-                    if (isMergeableValue(multiplyLeft, SExt32, true) && isMergeableValue(multiplyRight, SExt32, true)) 
+                    if (isMergeableValue(multiplyLeft, SExt32) && isMergeableValue(multiplyRight, SExt32))
                         return MultiplyNegSignExtend32;
                     // UMNEGL: d = -(ZExt32(n) * ZExt32(m))
-                    if (isMergeableValue(multiplyLeft, ZExt32, true) && isMergeableValue(multiplyRight, ZExt32, true)) 
+                    if (isMergeableValue(multiplyLeft, ZExt32) && isMergeableValue(multiplyRight, ZExt32))
                         return MultiplyNegZeroExtend32;
                     return Air::Oops;
@@ -2820 +2816 @@
                 if (isValidForm(newAirOpcode, Arg::Tmp, Arg::Tmp, Arg::Tmp)) {
                     append(newAirOpcode, tmp(multiplyLeft->child(0)), tmp(multiplyRight->child(0)), tmp(m_value));
-                    commitInternal(multiplyLeft);
-                    commitInternal(multiplyRight);
                     commitInternal(m_value->child(0));
                     return true;
@@ -2843 +2837 @@
 
         case Mul: {
-            if (m_value->type() == Int64
-                && isValidForm(MultiplySignExtend32, Arg::Tmp, Arg::Tmp, Arg::Tmp)
-                && m_value->child(0)->opcode() == SExt32
-                && !m_locked.contains(m_value->child(0))) {
-                Value* opLeft = m_value->child(0);
-                Value* left = opLeft->child(0);
-                Value* opRight = m_value->child(1);
-                Value* right = nullptr;
-
-                if (opRight->opcode() == SExt32 && !m_locked.contains(opRight->child(0))) {
-                    right = opRight->child(0);
-                } else if (m_value->child(1)->isRepresentableAs<int32_t>() && !m_locked.contains(m_value->child(1))) {
-                    // We just use the 64-bit const int as a 32 bit const int directly
-                    right = opRight;
-                }
-
-                if (right) {
-                    append(MultiplySignExtend32, tmp(left), tmp(right), tmp(m_value));
-                    return;
-                }
-            }
-            appendBinOp<Mul32, Mul64, MulDouble, MulFloat, Commutative>(
-                m_value->child(0), m_value->child(1));
+            Value* left = m_value->child(0);
+            Value* right = m_value->child(1);
+
+            auto tryAppendMultiplyWithExtend = [&] () -> bool {
+                auto tryAirOpcode = [&] () -> Air::Opcode {
+                    if (m_value->type() != Int64)
+                        return Air::Oops;
+                    // SMULL: d = SExt32(n) * SExt32(m)
+                    if (isMergeableValue(left, SExt32) && isMergeableValue(right, SExt32)) 
+                        return MultiplySignExtend32;
+                    // UMULL: d = ZExt32(n) * ZExt32(m)
+                    if (isMergeableValue(left, ZExt32) && isMergeableValue(right, ZExt32)) 
+                        return MultiplyZeroExtend32;
+                    return Air::Oops;
+                };
+
+                Air::Opcode opcode = tryAirOpcode();
+                if (isValidForm(opcode, Arg::Tmp, Arg::Tmp, Arg::Tmp)) {
+                    append(opcode, tmp(left->child(0)), tmp(right->child(0)), tmp(m_value));
+                    return true;
+                }
+                return false;
+            };
+
+            if (tryAppendMultiplyWithExtend())
+                return;
+
+            appendBinOp<Mul32, Mul64, MulDouble, MulFloat, Commutative>(left, right);
             return;
         }
@@ -3003 +3002 @@
             auto tryAppendEXTR = [&] (Value* left, Value* right) -> bool {
                 Air::Opcode opcode = opcodeForType(ExtractRegister32, ExtractRegister64, m_value->type());
-                if (!isValidForm(opcode, Arg::Tmp, Arg::Tmp, Arg::Imm, Arg::Tmp)) 
+                if (!isValidForm(opcode, Arg::Tmp, Arg::Tmp, Arg::Imm, Arg::Tmp))
                     return false;
                 if (left->opcode() != Shl || left->child(0)->opcode() != BitAnd || right->opcode() != ZShr)
@@ -3209 +3208 @@
                         XorNotRightShift32, XorNotRightShift64, 
                         XorNotUnsignedRightShift32, XorNotUnsignedRightShift64);
-                    if (!isValidForm(opcode, Arg::Tmp, Arg::Tmp, Arg::Imm, Arg::Tmp)) 
+                    if (!isValidForm(opcode, Arg::Tmp, Arg::Tmp, Arg::Imm, Arg::Tmp))
                         return false;
                     Value* mValue = shiftValue->child(0);

trunk/Source/JavaScriptCore/b3/air/AirOpcode.opcodes

@@ -285 +285 @@
     Tmp, Tmp, Tmp
 
+arm64: MultiplyZeroExtend32 U:G:32, U:G:32, D:G:64
+    Tmp, Tmp, Tmp
+
 arm64: Div32 U:G:32, U:G:32, ZD:G:32
     Tmp, Tmp, Tmp

trunk/Source/JavaScriptCore/b3/testb3.h

@@ -940 +940 @@
 void testMulImmArg(int, int);
 void testMulArgs32(int, int);
-void testMulArgs32SignExtend(int, int);
+void testMulArgs32SignExtend();
+void testMulArgs32ZeroExtend();
 void testMulImm32SignExtend(const int, int);
 void testMulLoadTwice();

trunk/Source/JavaScriptCore/b3/testb3_2.cpp

@@ -1229 +1229 @@
 }
 
-void testMulArgs32SignExtend(int a, int b)
-{
-    Procedure proc;
-    if (proc.optLevel() < 1)
+void testMulArgs32SignExtend()
+{
+    Procedure proc;
+    if (proc.optLevel() < 2)
         return;
     BasicBlock* root = proc.addBlock();
@@ -1247 +1247 @@
 
     auto code = compileProc(proc);
-
-    CHECK(invoke<long int>(*code, a, b) == ((long int) a) * ((long int) b));
+    if (isARM64())
+        checkUsesInstruction(*code, "smull");
+
+    for (auto nOperand : int32Operands()) {
+        for (auto mOperand : int32Operands()) {
+            int32_t n = nOperand.value;
+            int32_t m = mOperand.value;
+            CHECK_EQ(invoke<int64_t>(*code, n, m), static_cast<int64_t>(n) * static_cast<int64_t>(m));
+        }
+    }
+}
+
+void testMulArgs32ZeroExtend()
+{
+    Procedure proc;
+    if (proc.optLevel() < 2)
+        return;
+    BasicBlock* root = proc.addBlock();
+    Value* arg1 = root->appendNew<Value>(
+        proc, Trunc, Origin(),
+        root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0));
+    Value* arg2 = root->appendNew<Value>(
+        proc, Trunc, Origin(),
+        root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR1));
+    Value* left = root->appendNew<Value>(proc, ZExt32, Origin(), arg1);
+    Value* right = root->appendNew<Value>(proc, ZExt32, Origin(), arg2);
+    Value* mul = root->appendNew<Value>(proc, Mul, Origin(), left, right);
+    root->appendNewControlValue(proc, Return, Origin(), mul);
+
+    auto code = compileProc(proc);
+    if (isARM64())
+        checkUsesInstruction(*code, "umull");
+
+    for (auto nOperand : int32Operands()) {
+        for (auto mOperand : int32Operands()) {
+            uint32_t n = nOperand.value;
+            uint32_t m = mOperand.value;
+            CHECK_EQ(invoke<uint64_t>(*code, n, m), static_cast<uint64_t>(n) * static_cast<uint64_t>(m));
+        }
+    }
 }
 

trunk/Source/JavaScriptCore/b3/testb3_3.cpp

@@ -3731 +3731 @@
     RUN(testMulArgs32(0xFFFFFFFF, 0xFFFFFFFF));
     RUN(testMulArgs32(0xFFFFFFFE, 0xFFFFFFFF));
-    RUN(testMulArgs32SignExtend(1, 1));
-    RUN(testMulArgs32SignExtend(1, 2));
-    RUN(testMulArgs32SignExtend(0xFFFFFFFF, 0xFFFFFFFF));
-    RUN(testMulArgs32SignExtend(0xFFFFFFFE, 0xFFFFFFFF));
+    RUN(testMulArgs32SignExtend());
+    RUN(testMulArgs32ZeroExtend());
     RUN(testMulLoadTwice());
     RUN(testMulAddArgsLeft());