Attachment 382616 Details for Bug 203752 – Patch

[patch] Patch

patchOfflineAssembler (text/plain), 5.36 KB, created by Robin Morisset on 2019-11-01 13:40:48 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Robin Morisset

Created: 2019-11-01 13:40:48 PDT

Size: 5.36 KB

patch

obsolete

>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 66194e773bd..69ad07f5ae4 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-11-01  Robin Morisset  <rmorisset@apple.com>
>+
>+        The offline assembler is wrong about which immediates are supported by and/or/xor on ARM64
>+        https://bugs.webkit.org/show_bug.cgi?id=203752
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        See https://dinfuehr.github.io/blog/encoding-of-immediate-values-on-aarch64/ for the details of which immediates are supported.
>+        This patch is a minimal fix, ideally we should refactor all of the code dealing with immediates in risc.rb, but considering that I don't know ruby and this code is poorly/not tested, I went for the simplest possible fix.
>+
>+        * offlineasm/arm64.rb:
>+        * offlineasm/mips.rb:
>+        * offlineasm/risc.rb:
>+
> 2019-10-31  Tadeu Zagallo  <tzagallo@apple.com>
> 
>         Unreviewed, fix cloop builds after r251886
>diff --git a/Source/JavaScriptCore/offlineasm/arm.rb b/Source/JavaScriptCore/offlineasm/arm.rb
>index 33d4c93c88f..b23f65b8683 100644
>--- a/Source/JavaScriptCore/offlineasm/arm.rb
>+++ b/Source/JavaScriptCore/offlineasm/arm.rb
>@@ -300,7 +300,7 @@ class Sequence
>         }
>         result = riscLowerMalformedAddressesDouble(result)
>         result = riscLowerMisplacedImmediates(result, ["storeb", "storei", "storep", "storeq"])
>-        result = riscLowerMalformedImmediates(result, 0..0xff)
>+        result = riscLowerMalformedImmediates(result, 0..0xff, 0..0x0ff)
>         result = riscLowerMisplacedAddresses(result)
>         result = riscLowerRegisterReuse(result)
>         result = assignRegistersToTemporaries(result, :gpr, ARM_EXTRA_GPRS)
>diff --git a/Source/JavaScriptCore/offlineasm/arm64.rb b/Source/JavaScriptCore/offlineasm/arm64.rb
>index 6f26fb80835..0e29e7df632 100644
>--- a/Source/JavaScriptCore/offlineasm/arm64.rb
>+++ b/Source/JavaScriptCore/offlineasm/arm64.rb
>@@ -418,8 +418,41 @@ class Sequence
>                 false
>             end
>         }
>+
>         result = riscLowerMisplacedImmediates(result, ["storeb", "storei", "storep", "storeq"])
>-        result = riscLowerMalformedImmediates(result, 0..4095)
>+
>+        # The rules for which immediates are valid for and/or/xor instructions are fairly involved, see https://dinfuehr.github.io/blog/encoding-of-immediate-values-on-aarch64/
>+        validLogicalImmediates = []
>+        def rotate(value, n, size)
>+            mask = (1 << size) - 1
>+            shiftedValue = value << n
>+            result = (shiftedValue & mask) | ((shiftedValue & ~mask) >> size)
>+            return result
>+        end
>+        def replicate(value, size)
>+            until size == 64 do
>+                value = value | (value << size)
>+                size *= 2
>+            end
>+            return value
>+        end
>+        size = 2
>+        until size > 64 do
>+            for numberOfOnes in 1..(size-1) do
>+                for rotation in 0..(size-1) do
>+                    immediate = 0;
>+                    for i in 0..(numberOfOnes-1) do
>+                        immediate = immediate*2 + 1
>+                    end
>+                    immediate = rotate(immediate, rotation, size)
>+                    immediate = replicate(immediate, size)
>+                    validLogicalImmediates << immediateBase
>+                end
>+            end
>+            size *= 2
>+        end
>+        result = riscLowerMalformedImmediates(result, 0..4095, validLogicalImmediates)
>+
>         result = riscLowerMisplacedAddresses(result)
>         result = riscLowerMalformedAddresses(result) {
>             | node, address |
>diff --git a/Source/JavaScriptCore/offlineasm/mips.rb b/Source/JavaScriptCore/offlineasm/mips.rb
>index 56bc73ebaa0..b5f07b168c3 100644
>--- a/Source/JavaScriptCore/offlineasm/mips.rb
>+++ b/Source/JavaScriptCore/offlineasm/mips.rb
>@@ -729,7 +729,7 @@ class Sequence
>         result = riscLowerMalformedAddressesDouble(result)
>         result = riscLowerMisplacedImmediates(result, ["storeb", "storei", "storep"])
>         result = mipsLowerMisplacedImmediates(result)
>-        result = riscLowerMalformedImmediates(result, -0x7fff..0x7fff)
>+        result = riscLowerMalformedImmediates(result, -0x7fff..0x7fff, -0x7fff..0x7fff)
>         result = mipsLowerMisplacedAddresses(result)
>         result = riscLowerMisplacedAddresses(result)
>         result = riscLowerRegisterReuse(result)
>diff --git a/Source/JavaScriptCore/offlineasm/risc.rb b/Source/JavaScriptCore/offlineasm/risc.rb
>index 8b8de8d128a..75d7a98f33f 100644
>--- a/Source/JavaScriptCore/offlineasm/risc.rb
>+++ b/Source/JavaScriptCore/offlineasm/risc.rb
>@@ -362,7 +362,7 @@ class Immediate
>     end
> end
> 
>-def riscLowerMalformedImmediates(list, validImmediates)
>+def riscLowerMalformedImmediates(list, validImmediates, validLogicalImmediates)
>     newList = []
>     list.each {
>         | node |
>@@ -395,6 +395,8 @@ def riscLowerMalformedImmediates(list, validImmediates)
>                 else
>                     newList << node.riscLowerMalformedImmediatesRecurse(newList, validImmediates)
>                 end
>+            when "ori", "orh", "orp", "oris", "xori", "xorp", "andi", "andp"
>+                newList << node.riscLowerMalformedImmediatesRecurse(newList, validLogicalImmediates)
>             else
>                 newList << node.riscLowerMalformedImmediatesRecurse(newList, validImmediates)
>             end

Actions: View | Formatted Diff | Diff

Attachments on bug 203752: 382616 | 382618 | 382626