From e3c09488c63b82d3bb2274392e310494f50ff64c Mon Sep 17 00:00:00 2001 From: Bob McWhirter Date: Mon, 20 May 2019 16:10:44 -0400 Subject: [PATCH] Cleanup on aisle aarch64. Renaming methods to better match what they do. Renaming classes to better express what they do. --- .../asm/aarch64/AArch64Assembler.java | 14 ++++----- .../asm/aarch64/AArch64MacroAssembler.java | 31 ++++++++++--------- .../AArch64CGlobalDataLoadAddressOp.java | 2 +- ...va => SingleInstructionHostedPatcher.java} | 30 +++++++++--------- 4 files changed, 39 insertions(+), 38 deletions(-) mode change 100644 => 100755 compiler/src/org.graalvm.compiler.asm.aarch64/src/org/graalvm/compiler/asm/aarch64/AArch64Assembler.java mode change 100644 => 100755 compiler/src/org.graalvm.compiler.asm.aarch64/src/org/graalvm/compiler/asm/aarch64/AArch64MacroAssembler.java mode change 100644 => 100755 substratevm/src/com.oracle.svm.core.graal/src/com/oracle/svm/core/graal/code/aarch64/AArch64CGlobalDataLoadAddressOp.java rename substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/aarch64/{AArch64HostedPatcher.java => SingleInstructionHostedPatcher.java} (89%) mode change 100644 => 100755 diff --git a/compiler/src/org.graalvm.compiler.asm.aarch64/src/org/graalvm/compiler/asm/aarch64/AArch64Assembler.java b/compiler/src/org.graalvm.compiler.asm.aarch64/src/org/graalvm/compiler/asm/aarch64/AArch64Assembler.java old mode 100644 new mode 100755 index a45450c5f207..20dd9ed7df6b --- a/compiler/src/org.graalvm.compiler.asm.aarch64/src/org/graalvm/compiler/asm/aarch64/AArch64Assembler.java +++ b/compiler/src/org.graalvm.compiler.asm.aarch64/src/org/graalvm/compiler/asm/aarch64/AArch64Assembler.java @@ -2963,17 +2963,17 @@ public void msr(SystemRegister systemRegister, Register src) { public void annotatePatchingImmediate(int pos, Instruction instruction, int operandSizeBits, int offsetBits, int shift) { if (codePatchingAnnotationConsumer != null) { - codePatchingAnnotationConsumer.accept(new OperandDataAnnotation(pos, instruction, operandSizeBits, offsetBits, shift)); + codePatchingAnnotationConsumer.accept(new SingleInstructionAnnotation(pos, instruction, operandSizeBits, offsetBits, shift)); } } - void annotatePatchingImmediateNativeAddress(int pos, int operandSizeBits, int numInstrs) { + void annotateImmediateMovSequence(int pos, int numInstrs) { if (codePatchingAnnotationConsumer != null) { - codePatchingAnnotationConsumer.accept(new MovSequenceAnnotation(pos, operandSizeBits, numInstrs)); + codePatchingAnnotationConsumer.accept(new MovSequenceAnnotation(pos, numInstrs)); } } - public static class OperandDataAnnotation extends CodeAnnotation { + public static class SingleInstructionAnnotation extends CodeAnnotation { /** * The size of the operand, in bytes. @@ -2983,7 +2983,7 @@ public static class OperandDataAnnotation extends CodeAnnotation { public final Instruction instruction; public final int shift; - OperandDataAnnotation(int instructionPosition, Instruction instruction, int operandSizeBits, int offsetBits, int shift) { + SingleInstructionAnnotation(int instructionPosition, Instruction instruction, int operandSizeBits, int offsetBits, int shift) { super(instructionPosition); this.operandSizeBits = operandSizeBits; this.offsetBits = offsetBits; @@ -2997,12 +2997,10 @@ public static class MovSequenceAnnotation extends CodeAnnotation { /** * The size of the operand, in bytes. */ - public final int operandSizeBits; public final int numInstrs; - MovSequenceAnnotation(int instructionPosition, int operandSizeBits, int numInstrs) { + MovSequenceAnnotation(int instructionPosition, int numInstrs) { super(instructionPosition); - this.operandSizeBits = operandSizeBits; this.numInstrs = numInstrs; } } diff --git a/compiler/src/org.graalvm.compiler.asm.aarch64/src/org/graalvm/compiler/asm/aarch64/AArch64MacroAssembler.java b/compiler/src/org.graalvm.compiler.asm.aarch64/src/org/graalvm/compiler/asm/aarch64/AArch64MacroAssembler.java old mode 100644 new mode 100755 index 04211961b9d9..1760d3b3a438 --- a/compiler/src/org.graalvm.compiler.asm.aarch64/src/org/graalvm/compiler/asm/aarch64/AArch64MacroAssembler.java +++ b/compiler/src/org.graalvm.compiler.asm.aarch64/src/org/graalvm/compiler/asm/aarch64/AArch64MacroAssembler.java @@ -339,10 +339,13 @@ public void mov(int size, Register dst, Register src) { * Generates a 64-bit immediate move code sequence. * * @param dst general purpose register. May not be null, stackpointer or zero-register. - * @param imm + * @param imm the value to move into the register + * @param annotateImm Flag denoting if annotation should be added. */ - private void mov64(Register dst, long imm) { + private void mov64(Register dst, long imm, boolean annotateImm) { // We have to move all non zero parts of the immediate in 16-bit chunks + int numMovs = 0; + int pos = position(); boolean firstMove = true; for (int offset = 0; offset < 64; offset += 16) { int chunk = (int) (imm >> offset) & NumUtil.getNbitNumberInt(16); @@ -355,8 +358,12 @@ private void mov64(Register dst, long imm) { } else { movk(64, dst, chunk, offset); } + ++numMovs; } assert !firstMove; + if (annotateImm) { + annotateImmediateMovSequence(pos, numMovs); + } } /** @@ -378,7 +385,6 @@ public void mov(Register dst, long imm) { */ public void mov(Register dst, long imm, boolean annotateImm) { assert dst.getRegisterCategory().equals(CPU); - int pos = position(); if (imm == 0L) { movx(dst, zr); } else if (LogicalImmediateTable.isRepresentable(true, imm) != LogicalImmediateTable.Representable.NO) { @@ -391,10 +397,7 @@ public void mov(Register dst, long imm, boolean annotateImm) { mov(dst, (int) imm); sxt(64, 32, dst, dst); } else { - mov64(dst, imm); - if (annotateImm) { - annotatePatchingImmediateNativeAddress(pos, 64, 4); - } + mov64(dst, imm, annotateImm); } } @@ -448,7 +451,7 @@ public void movNativeAddress(Register dst, long imm, boolean annotateImm) { } } if (annotateImm) { - annotatePatchingImmediateNativeAddress(pos, 48, 3); + annotateImmediateMovSequence(pos, 3); } assert !firstMove; } @@ -1805,24 +1808,24 @@ public interface MacroInstruction { } /** - * Emits elf patchable adrp add sequence. + * Emits elf patchable adrp ldr sequence. */ - public void adrAddRel(int srcSize, Register result, AArch64Address a) { + public void adrpLdr(int srcSize, Register result, AArch64Address a) { if (codePatchingAnnotationConsumer != null) { - codePatchingAnnotationConsumer.accept(new ADRADDPRELMacroInstruction(position())); + codePatchingAnnotationConsumer.accept(new AdrpLdrMacroInstruction(position())); } super.adrp(a.getBase()); this.ldr(srcSize, result, a); } - public static class ADRADDPRELMacroInstruction extends CodeAnnotation implements MacroInstruction { - public ADRADDPRELMacroInstruction(int position) { + public static class AdrpLdrMacroInstruction extends CodeAnnotation implements MacroInstruction { + public AdrpLdrMacroInstruction(int position) { super(position); } @Override public String toString() { - return "ADR_PREL_PG"; + return "ADRP_LDR"; } @Override diff --git a/substratevm/src/com.oracle.svm.core.graal/src/com/oracle/svm/core/graal/code/aarch64/AArch64CGlobalDataLoadAddressOp.java b/substratevm/src/com.oracle.svm.core.graal/src/com/oracle/svm/core/graal/code/aarch64/AArch64CGlobalDataLoadAddressOp.java old mode 100644 new mode 100755 index bd1ec71af9b0..53aeee5723c0 --- a/substratevm/src/com.oracle.svm.core.graal/src/com/oracle/svm/core/graal/code/aarch64/AArch64CGlobalDataLoadAddressOp.java +++ b/substratevm/src/com.oracle.svm.core.graal/src/com/oracle/svm/core/graal/code/aarch64/AArch64CGlobalDataLoadAddressOp.java @@ -64,7 +64,7 @@ public void emitCode(CompilationResultBuilder crb, AArch64MacroAssembler masm) { // Pure symbol reference: the data contains the symbol's address, load it Register resultRegister = asRegister(result); AArch64Address address = AArch64Address.createScaledImmediateAddress(resultRegister, 0x0); - masm.adrAddRel(64, resultRegister, address); + masm.adrpLdr(64, resultRegister, address); crb.compilationResult.recordDataPatch(before, new CGlobalDataReference(dataInfo)); } else { // Data: load its address diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/aarch64/AArch64HostedPatcher.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/aarch64/SingleInstructionHostedPatcher.java old mode 100644 new mode 100755 similarity index 89% rename from substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/aarch64/AArch64HostedPatcher.java rename to substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/aarch64/SingleInstructionHostedPatcher.java index 2890d77dee68..848366eb412f --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/aarch64/AArch64HostedPatcher.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/aarch64/SingleInstructionHostedPatcher.java @@ -28,7 +28,7 @@ import org.graalvm.compiler.asm.Assembler.CodeAnnotation; import org.graalvm.compiler.asm.aarch64.AArch64Assembler; -import org.graalvm.compiler.asm.aarch64.AArch64Assembler.OperandDataAnnotation; +import org.graalvm.compiler.asm.aarch64.AArch64Assembler.SingleInstructionAnnotation; import org.graalvm.compiler.asm.aarch64.AArch64MacroAssembler; import org.graalvm.compiler.code.CompilationResult; import org.graalvm.nativeimage.hosted.Feature; @@ -62,12 +62,12 @@ public Consumer newConsumer(CompilationResult compilationResult) return new Consumer() { @Override public void accept(CodeAnnotation annotation) { - if (annotation instanceof OperandDataAnnotation) { - compilationResult.addAnnotation(new AArch64HostedPatcher(annotation.instructionPosition, (OperandDataAnnotation) annotation)); + if (annotation instanceof SingleInstructionAnnotation) { + compilationResult.addAnnotation(new SingleInstructionHostedPatcher(annotation.instructionPosition, (SingleInstructionAnnotation) annotation)); } else if (annotation instanceof AArch64Assembler.MovSequenceAnnotation) { - compilationResult.addAnnotation(new AArch64MovSequenceHostedPatcher(annotation.instructionPosition, (AArch64Assembler.MovSequenceAnnotation) annotation)); - } else if (annotation instanceof AArch64MacroAssembler.ADRADDPRELMacroInstruction) { - compilationResult.addAnnotation(new ADRADDPRELMacroInstructionHostedPatcher((AArch64MacroAssembler.ADRADDPRELMacroInstruction) annotation)); + compilationResult.addAnnotation(new MovSequenceHostedPatcher(annotation.instructionPosition, (AArch64Assembler.MovSequenceAnnotation) annotation)); + } else if (annotation instanceof AArch64MacroAssembler.AdrpLdrMacroInstruction) { + compilationResult.addAnnotation(new AdrpLdrMacroInstructionHostedPatcher((AArch64MacroAssembler.AdrpLdrMacroInstruction) annotation)); } else if (annotation instanceof AArch64MacroAssembler.AdrpAddMacroInstruction) { compilationResult.addAnnotation(new AdrpAddMacroInstructionHostedPatcher((AArch64MacroAssembler.AdrpAddMacroInstruction) annotation)); } @@ -78,10 +78,10 @@ public void accept(CodeAnnotation annotation) { } } -public class AArch64HostedPatcher extends CompilationResult.CodeAnnotation implements HostedPatcher { - private final OperandDataAnnotation annotation; +public class SingleInstructionHostedPatcher extends CompilationResult.CodeAnnotation implements HostedPatcher { + private final SingleInstructionAnnotation annotation; - public AArch64HostedPatcher(int instructionStartPosition, OperandDataAnnotation annotation) { + public SingleInstructionHostedPatcher(int instructionStartPosition, SingleInstructionAnnotation annotation) { super(instructionStartPosition); this.annotation = annotation; } @@ -145,10 +145,10 @@ public void relocate(Reference ref, RelocatableBuffer relocs, int compStart) { } } -class ADRADDPRELMacroInstructionHostedPatcher extends CompilationResult.CodeAnnotation implements HostedPatcher { - private final AArch64MacroAssembler.ADRADDPRELMacroInstruction macroInstruction; +class AdrpLdrMacroInstructionHostedPatcher extends CompilationResult.CodeAnnotation implements HostedPatcher { + private final AArch64MacroAssembler.AdrpLdrMacroInstruction macroInstruction; - ADRADDPRELMacroInstructionHostedPatcher(AArch64MacroAssembler.ADRADDPRELMacroInstruction macroInstruction) { + AdrpLdrMacroInstructionHostedPatcher(AArch64MacroAssembler.AdrpLdrMacroInstruction macroInstruction) { super(macroInstruction.instructionPosition); this.macroInstruction = macroInstruction; } @@ -201,10 +201,10 @@ public boolean equals(Object obj) { } } -class AArch64MovSequenceHostedPatcher extends CompilationResult.CodeAnnotation implements HostedPatcher { +class MovSequenceHostedPatcher extends CompilationResult.CodeAnnotation implements HostedPatcher { private final AArch64Assembler.MovSequenceAnnotation annotation; - AArch64MovSequenceHostedPatcher(int instructionStartPosition, AArch64Assembler.MovSequenceAnnotation annotation) { + MovSequenceHostedPatcher(int instructionStartPosition, AArch64Assembler.MovSequenceAnnotation annotation) { super(instructionStartPosition); this.annotation = annotation; } @@ -215,7 +215,7 @@ public void patch(int codePos, int relative, byte[] code) { int curValue = relative - (4 * annotation.numInstrs); // n 32-bit instrs to patch n 16-bit // movs - int bitsRemaining = annotation.operandSizeBits; + int bitsRemaining = annotation.numInstrs * 8; for (int i = 0; i < 4 * annotation.numInstrs; i = i + 4) { if (bitsRemaining >= 8) {