Skip to content
Permalink
Browse files
Change offlineasm to emit more efficient LLInt code.
https://bugs.webkit.org/show_bug.cgi?id=241856

Reviewed by Yusuke Suzuki.

1. Ruby treats numeric 0 as truthy.  However, there's a test in arm64LowerMalformedLoadStoreAddresses
   which assumes a value of 0 would be false.  As a result, we see offlineasm emit inefficient LLInt
   code like this:
    ".loc 3 821\n"        "movz x16, #0 \n"                    // LowLevelInterpreter64.asm:821
                          "add x13, x3, x16 \n"
                          "ldr x0, [x13] \n"

  ...  instead of this:
    ".loc 3 821\n"        "ldr x0, [x3] \n"                    // LowLevelInterpreter64.asm:821

   This patch fixes this.

2. offlineasm's emitARM64MoveImmediate chooses to use `movn` instead of `movz` based on whether a
   64-bit value is negative or not.  Instead, it should be making that decision based on the number of
   halfwords (16-bits) in the value that is 0xffff vs 0.  As a result, offlineasm emits code like this:
    ".loc 1 1638\n"       "movn x27, #1, lsl #48 \n"           // LowLevelInterpreter.asm:1638
                          "movk x27, #0, lsl #32 \n"
                          "movk x27, #0, lsl #16 \n"
                          "movk x27, #0 \n"

  ...  instead of this:
    ".loc 1 1638\n"       "movz x27, #65534, lsl #48 \n"       // LowLevelInterpreter.asm:1638

   This patch fixes this.

3. offlineasm is trivially assuming the range of immediate offsets for ldr/str instructions is
   [-255..4095].  However, that's only the range for byte sized load-stores.  For 32-bit, the range
   is actually [-255..16380].  For 64-bit, the range is actually [-255..32760].  As a result,
    offlineasm emits code like this:
    ".loc 1 633\n"        "movn x16, #16383 \n"                // LowLevelInterpreter.asm:633
    ".loc 1 1518\n"       "and x3, x3, x16 \n"                 // LowLevelInterpreter.asm:1518
    ".loc 1 1519\n"       "movz x16, #16088 \n"                // LowLevelInterpreter.asm:1519
                          "add x17, x3, x16 \n"
                          "ldr x3, [x17] \n"

  ...  instead of this:
    ".loc 1 633\n"        "movn x17, #16383 \n"                // LowLevelInterpreter.asm:633
    ".loc 1 1518\n"       "and x3, x3, x17 \n"                 // LowLevelInterpreter.asm:1518
    ".loc 1 1519\n"       "ldr x3, [x3, #16088] \n"            // LowLevelInterpreter.asm:1519

   This patch fixes this for 64-bit and 32-bit load-stores.  16-bit load-stores also has a wider
   range, but for now, it will continue to use the conservative range.

   This patch also introduces an `isMalformedArm64LoadAStoreAddress` so that this range check can be
   done consistently in all the places that checks for it.

4. offlineasm is eagerly emitting no-op arguments in instructions, e.g. "lsl #0", and adding 0.
   As a result, offlineasm emits code like this:
    ".loc 3 220\n"        "movz x13, #51168, lsl #0 \n"        // LowLevelInterpreter64.asm:220
                          "add x17, x1, x13, lsl #0 \n"
                          "ldr w4, [x17, #0] \n"

  ...  instead of this:
    ".loc 3 220\n"        "movz x13, #51168 \n"                // LowLevelInterpreter64.asm:220
                          "add x17, x1, x13 \n"
                          "ldr w4, [x17] \n"

   This unnecessary arguments are actually very common throughout the emitted LLIntAssembly.h.

   This patch removes these unnecessary arguments, which makes the emitted LLInt code more human
   readable due to less clutter.

This patch has passed the testapi and JSC stress tests with a Release build on an M1 Mac.

I also manually verified that the emitARM64MoveImmediate code is working properly by
hacking up LowLevelInterpreter64.asm to emit moves of constants of different values in
the ranges, and for load-store instructions of different sizes, and visually inspecting
the emitted code.

* Source/JavaScriptCore/offlineasm/arm64.rb:

Canonical link: https://commits.webkit.org/251771@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295766 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Mark Lam committed Jun 23, 2022
1 parent 69702d9 commit daa5f7a102774147288388f3a5553326a3b72f45
Showing 1 changed file with 77 additions and 28 deletions.
@@ -227,8 +227,21 @@ def arm64Operand(kind)

class Address
def arm64Operand(kind)
raise "Invalid offset #{offset.value} at #{codeOriginString}" if offset.value < -255 or offset.value > 4095
"[#{base.arm64Operand(:quad)}, \##{offset.value}]"
case kind
when :quad, :ptr, :double
if $currentSettings["ADDRESS64"]
raise "Invalid offset #{offset.value} at #{codeOriginString}" if offset.value < -255 or offset.value > 32760
else
raise "Invalid offset #{offset.value} at #{codeOriginString}" if offset.value < -255 or offset.value > 16380
end
when :word, :int, :float
raise "Invalid offset #{offset.value} at #{codeOriginString}" if offset.value < -255 or offset.value > 16380
else
raise "Invalid offset #{offset.value} at #{codeOriginString}" if offset.value < -255 or offset.value > 4095
end
offset.value.zero? \
? "[#{base.arm64Operand(:quad)}]"
: "[#{base.arm64Operand(:quad)}, \##{offset.value}]"
end

def arm64SimpleAddressOperand(kind)
@@ -237,18 +250,24 @@ def arm64SimpleAddressOperand(kind)
end

def arm64EmitLea(destination, kind)
$asm.puts "add #{destination.arm64Operand(kind)}, #{base.arm64Operand(kind)}, \##{offset.value}"
offset.value.zero? \
? ($asm.puts "mov #{destination.arm64Operand(kind)}, #{base.arm64Operand(kind)}")
: ($asm.puts "add #{destination.arm64Operand(kind)}, #{base.arm64Operand(kind)}, \##{offset.value}")
end
end

class BaseIndex
def arm64Operand(kind)
raise "Invalid offset #{offset.value} at #{codeOriginString}" if offset.value != 0
"[#{base.arm64Operand(:quad)}, #{index.arm64Operand(:quad)}, lsl \##{scaleShift}]"
scaleShift.zero? \
? "[#{base.arm64Operand(:quad)}, #{index.arm64Operand(:quad)}]"
: "[#{base.arm64Operand(:quad)}, #{index.arm64Operand(:quad)}, lsl \##{scaleShift}]"
end

def arm64EmitLea(destination, kind)
$asm.puts "add #{destination.arm64Operand(kind)}, #{base.arm64Operand(kind)}, #{index.arm64Operand(kind)}, lsl \##{scaleShift}"
scaleShift.zero? \
? ($asm.puts "add #{destination.arm64Operand(kind)}, #{base.arm64Operand(kind)}, #{index.arm64Operand(kind)}")
: ($asm.puts "add #{destination.arm64Operand(kind)}, #{base.arm64Operand(kind)}, #{index.arm64Operand(kind)}, lsl \##{scaleShift}")
end
end

@@ -264,29 +283,42 @@ def arm64Operand(kind)
# Actual lowering code follows.
#

def arm64LowerMalformedLoadStoreAddresses(list)
newList = []

def isAddressMalformed(opcode, operand)
malformed = false
if operand.is_a? Address
malformed ||= (not (-255..4095).include? operand.offset.value)
if opcode =~ /q$/ and $currentSettings["ADDRESS64"]
malformed ||= operand.offset.value % 8
def isMalformedArm64LoadAStoreAddress(opcode, operand)
malformed = false
if operand.is_a? Address
case opcode
when "loadp", "storep", "loadq", "storeq"
if $currentSettings["ADDRESS64"]
malformed ||= (not (-255..32760).include? operand.offset.value)
malformed ||= (not (operand.offset.value % 8).zero?)
else
malformed ||= (not (-255..16380).include? operand.offset.value)
end
when "loadd", "stored"
malformed ||= (not (-255..32760).include? operand.offset.value)
malformed ||= (not (operand.offset.value % 8).zero?)
when "loadi", "loadis", "storei"
malformed ||= (not (-255..16380).include? operand.offset.value)
else
# This is just a conservative estimate of the max offset.
malformed ||= (not (-255..4095).include? operand.offset.value)
end
malformed
end
malformed
end

def arm64LowerMalformedLoadStoreAddresses(list)
newList = []

list.each {
| node |
if node.is_a? Instruction
if node.opcode =~ /^store/ and isAddressMalformed(node.opcode, node.operands[1])
if node.opcode =~ /^store/ and isMalformedArm64LoadAStoreAddress(node.opcode, node.operands[1])
address = node.operands[1]
tmp = Tmp.new(codeOrigin, :gpr)
newList << Instruction.new(node.codeOrigin, "move", [address.offset, tmp])
newList << Instruction.new(node.codeOrigin, node.opcode, [node.operands[0], BaseIndex.new(node.codeOrigin, address.base, tmp, Immediate.new(codeOrigin, 1), Immediate.new(codeOrigin, 0))], node.annotation)
elsif node.opcode =~ /^load/ and isAddressMalformed(node.opcode, node.operands[0])
elsif node.opcode =~ /^load/ and isMalformedArm64LoadAStoreAddress(node.opcode, node.operands[0])
address = node.operands[0]
tmp = Tmp.new(codeOrigin, :gpr)
newList << Instruction.new(node.codeOrigin, "move", [address.offset, tmp])
@@ -399,7 +431,7 @@ def getModifiedListARM64(result = @list)
address.offset.value == 0 and
(node.opcode =~ /^lea/ or address.scale == 1 or address.scale == size)
elsif address.is_a? Address
(-255..4095).include? address.offset.value
not isMalformedArm64LoadAStoreAddress(node.opcode, address)
else
false
end
@@ -443,10 +475,8 @@ def replicate(value, size)
result = riscLowerMalformedAddresses(result) {
| node, address |
case node.opcode
when /^load/
true
when /^store/
not (address.is_a? Address and address.offset.value < 0)
when /^load/, /^store/
not address.is_a? Address or not isMalformedArm64LoadAStoreAddress(node.opcode, address)
when /^lea/
true
when /^atomic/
@@ -655,21 +685,40 @@ def emitARM64Compare(operands, kind, compareCode)
end

def emitARM64MoveImmediate(value, target)
numberOfFilledHalfWords = 0
numberOfZeroHalfWords = 0
[48, 32, 16, 0].each {
| shift |
currentValue = (value >> shift) & 0xffff
if currentValue == 0xffff
numberOfFilledHalfWords += 1
end
if currentValue == 0
numberOfZeroHalfWords += 1
end
}
fillOtherHalfWordsWithOnes = (numberOfFilledHalfWords > numberOfZeroHalfWords)

first = true
isNegative = value < 0
[48, 32, 16, 0].each {
| shift |
currentValue = (value >> shift) & 0xffff
next if currentValue == (isNegative ? 0xffff : 0) and (shift != 0 or !first)
next if currentValue == (fillOtherHalfWordsWithOnes ? 0xffff : 0) and (shift != 0 or !first)
if first
if isNegative
$asm.puts "movn #{target.arm64Operand(:quad)}, \##{(~currentValue) & 0xffff}, lsl \##{shift}"
if fillOtherHalfWordsWithOnes
shift.zero? \
? ($asm.puts "movn #{target.arm64Operand(:quad)}, \##{(~currentValue) & 0xffff}")
: ($asm.puts "movn #{target.arm64Operand(:quad)}, \##{(~currentValue) & 0xffff}, lsl \##{shift}")
else
$asm.puts "movz #{target.arm64Operand(:quad)}, \##{currentValue}, lsl \##{shift}"
shift.zero? \
? ($asm.puts "movz #{target.arm64Operand(:quad)}, \##{currentValue}")
: ($asm.puts "movz #{target.arm64Operand(:quad)}, \##{currentValue}, lsl \##{shift}")
end
first = false
else
$asm.puts "movk #{target.arm64Operand(:quad)}, \##{currentValue}, lsl \##{shift}"
shift.zero? \
? ($asm.puts "movk #{target.arm64Operand(:quad)}, \##{currentValue}")
: ($asm.puts "movk #{target.arm64Operand(:quad)}, \##{currentValue}, lsl \##{shift}")
end
}
end

0 comments on commit daa5f7a

Please sign in to comment.