[PATCH] D149443: [ARM] add Thumb-1 8-bit movs/adds relocations to LLVM

[Ties Stuij via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdc49fbd2df7d: [ARM] add Thumb-1 8-bit movs/adds relocations 
to LLVM (authored by stuij).

Changed prior to commit:
  https://reviews.llvm.org/D149443?vs=533190&id=533630#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149443/new/

https://reviews.llvm.org/D149443

Files:
  llvm/include/llvm/BinaryFormat/ELFRelocs/ARM.def
  llvm/lib/Target/ARM/ARMInstrInfo.td
  llvm/lib/Target/ARM/ARMInstrThumb.td
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMFixupKinds.h
  llvm/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMMCExpr.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMMCExpr.h
  llvm/test/MC/ARM/negative-immediates-thumb1-fail.s
  llvm/test/MC/ARM/thumb-8-bit-relocs.s
  llvm/test/MC/ARM/thumb-diagnostics.s
  llvm/test/MC/ARM/thumb-fixups.s

Index: llvm/test/MC/ARM/thumb-fixups.s
===
--- /dev/null
+++ llvm/test/MC/ARM/thumb-fixups.s
@@ -0,0 +1,25 @@
+@ RUN: llvm-mc -triple armv6m-unknown-unknown %s --show-encoding -o - | \
+@ RUN:   FileCheck %s
+
+movs r3, :upper8_15:_foo
+adds r3, :upper0_7:_foo
+adds r3, :lower8_15:_foo
+adds r3, :lower0_7:_foo
+
+@ CHECK:  movsr3, :upper8_15:_foo @ encoding: [A,0x23]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_upper_8_15
+@ CHECK-NEXT: addsr3, :upper0_7:_foo  @ encoding: [A,0x33]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_upper_0_7
+@ CHECK-NEXT: addsr3, :lower8_15:_foo @ encoding: [A,0x33]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_lower_8_15
+@ CHECK-NEXT: addsr3, :lower0_7:_foo  @ encoding: [A,0x33]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_lower_0_7
+
+@ GNU syntax variants:
+movs r3, #:upper8_15:#_foo
+movs r3, #:upper8_15:_foo
+
+@ CHECK:  movsr3, :upper8_15:_foo @ encoding: [A,0x23]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_upper_8_15
+@ CHECK-NEXT: movsr3, :upper8_15:_foo @ encoding: [A,0x23]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_upper_8_15
Index: llvm/test/MC/ARM/thumb-diagnostics.s
===
--- llvm/test/MC/ARM/thumb-diagnostics.s
+++ llvm/test/MC/ARM/thumb-diagnostics.s
@@ -195,6 +195,34 @@
 @ CHECK-ERRORS: note: operand must be an immediate in the range [0,31]
 @ CHECK-ERRORS: note: too many operands for instruction
 
+@ Out of range immediates for MOVS/ADDS instruction.
+movs r3, #-1
+adds r3, #256
+@ CHECK-ERRORS: error: invalid instruction, any one of the following would fix this:
+@ CHECK-ERRORS-NEXT: movs r3, #-1
+@ CHECK-ERRORS-NEXT: ^
+@ CHECK-ERRORS: note: operand must be an immediate in the range [0,255] or a relocatable expression
+@ CHECK-ERRORS-NEXT: movs r3, #-1
+@ CHECK-ERRORS-NEXT:  ^
+@ CHECK-ERRORS: note: operand must be a register in range [r0, r7]
+@ CHECK-ERRORS-NEXT: movs r3, #-1
+@ CHECK-ERRORS-NEXT:  ^
+@ CHECK-ERRORS: error: invalid instruction, any one of the following would fix this:
+@ CHECK-ERRORS-NEXT: adds r3, #256
+@ CHECK-ERRORS-NEXT: ^
+@ CHECK-ERRORS: note: instruction requires: thumb2
+@ CHECK-ERRORS-NEXT: adds r3, #256
+@ CHECK-ERRORS-NEXT: ^
+@ CHECK-ERRORS: note: invalid operand for instruction
+@ CHECK-ERRORS-NEXT: adds r3, #256
+@ CHECK-ERRORS-NEXT:  ^
+@ CHECK-ERRORS-NEXT: note: operand must be an immediate in the range [0,255] or a relocatable expression
+@ CHECK-ERRORS-NEXT: adds r3, #256
+@ CHECK-ERRORS-NEXT:  ^
+@ CHECK-ERRORS-NEXT: note: operand must be a register in range [r0, r7]
+@ CHECK-ERRORS-NEXT: adds r3, #256
+@ CHECK-ERRORS-NEXT:  ^
+
 @ Mismatched source/destination operands for MUL instruction.
 muls r1, r2, r3
 @ CHECK-ERRORS: error: destination register must match source register
Index: llvm/test/MC/ARM/thumb-8-bit-relocs.s
===
--- /dev/null
+++ llvm/test/MC/ARM/thumb-8-bit-relocs.s
@@ -0,0 +1,35 @@
+@ RUN: llvm-mc -triple thumbv6m-eabi -o - %s | FileCheck %s
+@ RUN: llvm-mc -triple thumbv6m-eabi -filetype obj -o - %s | llvm-readobj -r - \
+@ RUN:   | FileCheck -check-prefix CHECK-RELOCATIONS %s
+@ RUN: llvm-mc -triple thumbv7m-eabi -o - %s | FileCheck %s
+@ RUN: llvm-mc -triple thumbv7m-eabi -filetype obj -o - %s | llvm-readobj -r - \
+@ RUN:   | FileCheck -check-prefix CHECK-RELOCATIONS %s
+
+.syntax unified
+
+.type function,%function
+function:
+  bx lr
+
+.g

[PATCH] D149443: [ARM] add Thumb-1 8-bit movs/adds relocations to LLVM

[Ties Stuij via Phabricator via cfe-commits]

stuij updated this revision to Diff 533190.
stuij added a comment.

address review comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149443/new/

https://reviews.llvm.org/D149443

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  llvm/include/llvm/BinaryFormat/ELFRelocs/ARM.def
  llvm/lib/Target/ARM/ARMInstrInfo.td
  llvm/lib/Target/ARM/ARMInstrThumb.td
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMFixupKinds.h
  llvm/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMMCExpr.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMMCExpr.h
  llvm/test/MC/ARM/negative-immediates-thumb1-fail.s
  llvm/test/MC/ARM/thumb-8-bit-relocs.s
  llvm/test/MC/ARM/thumb-diagnostics.s
  llvm/test/MC/ARM/thumb-fixups.s

Index: llvm/test/MC/ARM/thumb-fixups.s
===
--- /dev/null
+++ llvm/test/MC/ARM/thumb-fixups.s
@@ -0,0 +1,25 @@
+@ RUN: llvm-mc -triple armv6m-unknown-unknown %s --show-encoding -o - | \
+@ RUN:   FileCheck %s
+
+movs r3, :upper8_15:_foo
+adds r3, :upper0_7:_foo
+adds r3, :lower8_15:_foo
+adds r3, :lower0_7:_foo
+
+@ CHECK:  movsr3, :upper8_15:_foo @ encoding: [A,0x23]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_upper_8_15
+@ CHECK-NEXT: addsr3, :upper0_7:_foo  @ encoding: [A,0x33]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_upper_0_7
+@ CHECK-NEXT: addsr3, :lower8_15:_foo @ encoding: [A,0x33]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_lower_8_15
+@ CHECK-NEXT: addsr3, :lower0_7:_foo  @ encoding: [A,0x33]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_lower_0_7
+
+@ GNU syntax variants:
+movs r3, #:upper8_15:#_foo
+movs r3, #:upper8_15:_foo
+
+@ CHECK:  movsr3, :upper8_15:_foo @ encoding: [A,0x23]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_upper_8_15
+@ CHECK-NEXT: movsr3, :upper8_15:_foo @ encoding: [A,0x23]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_upper_8_15
Index: llvm/test/MC/ARM/thumb-diagnostics.s
===
--- llvm/test/MC/ARM/thumb-diagnostics.s
+++ llvm/test/MC/ARM/thumb-diagnostics.s
@@ -195,6 +195,34 @@
 @ CHECK-ERRORS: note: operand must be an immediate in the range [0,31]
 @ CHECK-ERRORS: note: too many operands for instruction
 
+@ Out of range immediates for MOVS/ADDS instruction.
+movs r3, #-1
+adds r3, #256
+@ CHECK-ERRORS: error: invalid instruction, any one of the following would fix this:
+@ CHECK-ERRORS-NEXT: movs r3, #-1
+@ CHECK-ERRORS-NEXT: ^
+@ CHECK-ERRORS: note: operand must be an immediate in the range [0,255] or a relocatable expression
+@ CHECK-ERRORS-NEXT: movs r3, #-1
+@ CHECK-ERRORS-NEXT:  ^
+@ CHECK-ERRORS: note: operand must be a register in range [r0, r7]
+@ CHECK-ERRORS-NEXT: movs r3, #-1
+@ CHECK-ERRORS-NEXT:  ^
+@ CHECK-ERRORS: error: invalid instruction, any one of the following would fix this:
+@ CHECK-ERRORS-NEXT: adds r3, #256
+@ CHECK-ERRORS-NEXT: ^
+@ CHECK-ERRORS: note: instruction requires: thumb2
+@ CHECK-ERRORS-NEXT: adds r3, #256
+@ CHECK-ERRORS-NEXT: ^
+@ CHECK-ERRORS: note: invalid operand for instruction
+@ CHECK-ERRORS-NEXT: adds r3, #256
+@ CHECK-ERRORS-NEXT:  ^
+@ CHECK-ERRORS-NEXT: note: operand must be an immediate in the range [0,255] or a relocatable expression
+@ CHECK-ERRORS-NEXT: adds r3, #256
+@ CHECK-ERRORS-NEXT:  ^
+@ CHECK-ERRORS-NEXT: note: operand must be a register in range [r0, r7]
+@ CHECK-ERRORS-NEXT: adds r3, #256
+@ CHECK-ERRORS-NEXT:  ^
+
 @ Mismatched source/destination operands for MUL instruction.
 muls r1, r2, r3
 @ CHECK-ERRORS: error: destination register must match source register
Index: llvm/test/MC/ARM/thumb-8-bit-relocs.s
===
--- /dev/null
+++ llvm/test/MC/ARM/thumb-8-bit-relocs.s
@@ -0,0 +1,35 @@
+@ RUN: llvm-mc -triple thumbv6m-eabi -o - %s | FileCheck %s
+@ RUN: llvm-mc -triple thumbv6m-eabi -filetype obj -o - %s | llvm-readobj -r - \
+@ RUN:   | FileCheck -check-prefix CHECK-RELOCATIONS %s
+@ RUN: llvm-mc -triple thumbv7m-eabi -o - %s | FileCheck %s
+@ RUN: llvm-mc -triple thumbv7m-eabi -filetype obj -o - %s | llvm-readobj -r - \
+@ RUN:   | FileCheck -check-prefix CHECK-RELOCATIONS %s
+
+.syntax unified
+
+.type function,%function
+function:
+  bx lr
+
+.global external
+.type external,%function
+
+.type test,%function
+test:
+  movs r3, :upper8_15:function
+  adds r3, :upper0_7:function
+  adds r3, :lower8_15:function
+  adds r3, :lower0_7

[PATCH] D149443: [ARM] add Thumb-1 8-bit movs/adds relocations to LLVM

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added inline comments.



Comment at: llvm/test/MC/ARM/thumb-fixups.s:1
+@ RUN: llvm-mc -triple armv6m-unknown-unknown %s --show-encoding > %t
+@ RUN: FileCheck < %t %s

We normally just pipe the output to FileCheck, e.g. `| FileCheck %s`



Comment at: llvm/test/MC/ARM/thumb-fixups.s:9
+
+@ CHECK: movsr3, :upper8_15:_foo @ encoding: [A,0x23]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: 
fixup_arm_thumb_upper_8_15




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149443/new/

https://reviews.llvm.org/D149443

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149443: [ARM] add Thumb-1 8-bit movs/adds relocations to LLVM

[Ties Stuij via Phabricator via cfe-commits]

stuij updated this revision to Diff 532685.
stuij marked 3 inline comments as done.
stuij added a comment.

addressed review comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149443/new/

https://reviews.llvm.org/D149443

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  llvm/include/llvm/BinaryFormat/ELFRelocs/ARM.def
  llvm/lib/Target/ARM/ARMInstrInfo.td
  llvm/lib/Target/ARM/ARMInstrThumb.td
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMFixupKinds.h
  llvm/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMMCExpr.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMMCExpr.h
  llvm/test/MC/ARM/negative-immediates-thumb1-fail.s
  llvm/test/MC/ARM/thumb-8-bit-relocs.s
  llvm/test/MC/ARM/thumb-diagnostics.s
  llvm/test/MC/ARM/thumb-fixups.s

Index: llvm/test/MC/ARM/thumb-fixups.s
===
--- /dev/null
+++ llvm/test/MC/ARM/thumb-fixups.s
@@ -0,0 +1,25 @@
+@ RUN: llvm-mc -triple armv6m-unknown-unknown %s --show-encoding > %t
+@ RUN: FileCheck < %t %s
+
+movs r3, :upper8_15:_foo
+adds r3, :upper0_7:_foo
+adds r3, :lower8_15:_foo
+adds r3, :lower0_7:_foo
+
+@ CHECK: movsr3, :upper8_15:_foo @ encoding: [A,0x23]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_upper_8_15
+@ CHECK-NEXT: addsr3, :upper0_7:_foo  @ encoding: [A,0x33]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_upper_0_7
+@ CHECK-NEXT: addsr3, :lower8_15:_foo @ encoding: [A,0x33]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_lower_8_15
+@ CHECK-NEXT: addsr3, :lower0_7:_foo  @ encoding: [A,0x33]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_lower_0_7
+
+@ GNU syntax variants:
+movs r3, #:upper8_15:#_foo
+movs r3, #:upper8_15:_foo
+
+@ CHECK: movsr3, :upper8_15:_foo @ encoding: [A,0x23]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_upper_8_15
+@ CHECK-NEXT: movsr3, :upper8_15:_foo @ encoding: [A,0x23]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_upper_8_15
Index: llvm/test/MC/ARM/thumb-diagnostics.s
===
--- llvm/test/MC/ARM/thumb-diagnostics.s
+++ llvm/test/MC/ARM/thumb-diagnostics.s
@@ -195,6 +195,34 @@
 @ CHECK-ERRORS: note: operand must be an immediate in the range [0,31]
 @ CHECK-ERRORS: note: too many operands for instruction
 
+@ Out of range immediates for MOVS/ADDS instruction.
+movs r3, #-1
+adds r3, #256
+@ CHECK-ERRORS: error: invalid instruction, any one of the following would fix this:
+@ CHECK-ERRORS-NEXT: movs r3, #-1
+@ CHECK-ERRORS-NEXT: ^
+@ CHECK-ERRORS: note: operand must be an immediate in the range [0,255] or a relocatable expression
+@ CHECK-ERRORS-NEXT: movs r3, #-1
+@ CHECK-ERRORS-NEXT:  ^
+@ CHECK-ERRORS: note: operand must be a register in range [r0, r7]
+@ CHECK-ERRORS-NEXT: movs r3, #-1
+@ CHECK-ERRORS-NEXT:  ^
+@ CHECK-ERRORS: error: invalid instruction, any one of the following would fix this:
+@ CHECK-ERRORS-NEXT: adds r3, #256
+@ CHECK-ERRORS-NEXT: ^
+@ CHECK-ERRORS: note: instruction requires: thumb2
+@ CHECK-ERRORS-NEXT: adds r3, #256
+@ CHECK-ERRORS-NEXT: ^
+@ CHECK-ERRORS: note: invalid operand for instruction
+@ CHECK-ERRORS-NEXT: adds r3, #256
+@ CHECK-ERRORS-NEXT:  ^
+@ CHECK-ERRORS-NEXT: note: operand must be an immediate in the range [0,255] or a relocatable expression
+@ CHECK-ERRORS-NEXT: adds r3, #256
+@ CHECK-ERRORS-NEXT:  ^
+@ CHECK-ERRORS-NEXT: note: operand must be a register in range [r0, r7]
+@ CHECK-ERRORS-NEXT: adds r3, #256
+@ CHECK-ERRORS-NEXT:  ^
+
 @ Mismatched source/destination operands for MUL instruction.
 muls r1, r2, r3
 @ CHECK-ERRORS: error: destination register must match source register
Index: llvm/test/MC/ARM/thumb-8-bit-relocs.s
===
--- /dev/null
+++ llvm/test/MC/ARM/thumb-8-bit-relocs.s
@@ -0,0 +1,35 @@
+@ RUN: llvm-mc -triple thumbv6m-eabi -o - %s | FileCheck %s
+@ RUN: llvm-mc -triple thumbv6m-eabi -filetype obj -o - %s | llvm-readobj -r - \
+@ RUN:   | FileCheck -check-prefix CHECK-RELOCATIONS %s
+@ RUN: llvm-mc -triple thumbv7m-eabi -o - %s | FileCheck %s
+@ RUN: llvm-mc -triple thumbv7m-eabi -filetype obj -o - %s | llvm-readobj -r - \
+@ RUN:   | FileCheck -check-prefix CHECK-RELOCATIONS %s
+
+.syntax unified
+
+.type function,%function
+function:
+  bx lr
+
+.global external
+.type external,%function
+
+.type test,%function
+test:
+  movs r3, :upper8_15:function
+  adds r3, :upper0_7:function
+  adds r3, :lower8_15

[PATCH] D149443: [ARM] add Thumb-1 8-bit movs/adds relocations to LLVM

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp:1244
+int64_t Value = CE->getValue();
+return Value >= 0 && Value < 256;
+  }

`isUInt<8>(Value)`.

Can the limit be tested?



Comment at: llvm/test/MC/ARM/thumb-8-bit-relocs.s:1
+@ RUN: llvm-mc -triple thumbv6m-eabi -filetype asm -o - %s | FileCheck %s
+@ RUN: llvm-mc -triple thumbv6m-eabi -filetype obj -o - %s | llvm-readobj -r - 
\

`-filetype asm` can be omitted.



Comment at: llvm/test/MC/ARM/thumb-8-bit-relocs.s:31
+@ CHECK-RELOCATIONS: Relocations [
+@ CHECK-RELOCATIONS: 0x2 R_ARM_THM_ALU_ABS_G3 function
+@ CHECK-RELOCATIONS: 0x4 R_ARM_THM_ALU_ABS_G2_NC function

Add `-NEXT` whenever applicable to test that there is no other relocation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149443/new/

https://reviews.llvm.org/D149443

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149443: [ARM] add Thumb-1 8-bit movs/adds relocations to LLVM

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp:6422
+  // consume an optional trailing '#' (GNU compatibility)
+  if (getLexer().is(AsmToken::Hash))
+Parser.Lex();

Use `parseOptionalToken`. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149443/new/

https://reviews.llvm.org/D149443

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149443: [ARM] add Thumb-1 8-bit movs/adds relocations to LLVM

[John Brawn via Phabricator via cfe-commits]

john.brawn accepted this revision.
john.brawn added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149443/new/

https://reviews.llvm.org/D149443

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149443: [ARM] add Thumb-1 8-bit movs/adds relocations to LLVM

[Ties Stuij via Phabricator via cfe-commits]

stuij added a comment.

In D149443#4403186 , @john.brawn 
wrote:

> A few comments:
>
> - Please run clang-format on the patch (pre-merge is showing as failed due to 
> this)
> - We should have a test to check that we're emitting the right relocation 
> (probably something similar to test/MC/ARM/thumb-movwt-reloc.s)
> - There should be something in ARMAsmParser::validateInstruction to check 
> that the expression is valid, like we do for movw/movt, as currently you can 
> do `movs r0, :lower16:some_symbol` and it's accepted and results in a 16-bit 
> mov with a movw relocation.
> - Something weird is happening when I try to use these expressions in movs on 
> v7m. If I try to assemble `movs r0, :upper8_15:some_symbol` I get "error: 
> expected relocatable expression", and looking at the `llvm-mc --show-encoding 
> --show-inst-operands` it's selected movs.w instead of 16-bit mov for some 
> reason.

the recent changes should have addressed all of these




Comment at: llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp:498-505
+  case ARM::fixup_arm_thumb_upper_8_15:
+return (Value & 0xff00) >> 24;
+  case ARM::fixup_arm_thumb_upper_0_7:
+return (Value & 0x00ff) >> 16;
+  case ARM::fixup_arm_thumb_lower_8_15:
+return (Value & 0xff00) >> 8;
+  case ARM::fixup_arm_thumb_lower_0_7:

john.brawn wrote:
> The calculation here isn't correct when IsResolved is false. In that case 
> we're calculating the addend for a relocation, and the value of that will be 
> `Value&0xff` for all four of these fixups (movw/movt has similar behaviour). 
done


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149443/new/

https://reviews.llvm.org/D149443

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149443: [ARM] add Thumb-1 8-bit movs/adds relocations to LLVM

[Ties Stuij via Phabricator via cfe-commits]

stuij updated this revision to Diff 531792.
stuij marked 2 inline comments as done.
stuij added a comment.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

addressed review comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149443/new/

https://reviews.llvm.org/D149443

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  llvm/include/llvm/BinaryFormat/ELFRelocs/ARM.def
  llvm/lib/Target/ARM/ARMInstrInfo.td
  llvm/lib/Target/ARM/ARMInstrThumb.td
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMFixupKinds.h
  llvm/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMMCExpr.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMMCExpr.h
  llvm/test/MC/ARM/negative-immediates-thumb1-fail.s
  llvm/test/MC/ARM/thumb-8-bit-relocs.s
  llvm/test/MC/ARM/thumb-fixups.s

Index: llvm/test/MC/ARM/thumb-fixups.s
===
--- /dev/null
+++ llvm/test/MC/ARM/thumb-fixups.s
@@ -0,0 +1,25 @@
+@ RUN: llvm-mc -triple armv6m-unknown-unknown %s --show-encoding > %t
+@ RUN: FileCheck < %t %s
+
+movs r3, :upper8_15:_foo
+adds r3, :upper0_7:_foo
+adds r3, :lower8_15:_foo
+adds r3, :lower0_7:_foo
+
+@ CHECK: movsr3, :upper8_15:_foo @ encoding: [A,0x23]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_upper_8_15
+@ CHECK-NEXT: addsr3, :upper0_7:_foo  @ encoding: [A,0x33]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_upper_0_7
+@ CHECK-NEXT: addsr3, :lower8_15:_foo @ encoding: [A,0x33]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_lower_8_15
+@ CHECK-NEXT: addsr3, :lower0_7:_foo  @ encoding: [A,0x33]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_lower_0_7
+
+@ GNU syntax variants:
+movs r3, #:upper8_15:#_foo
+movs r3, #:upper8_15:_foo
+
+@ CHECK: movsr3, :upper8_15:_foo @ encoding: [A,0x23]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_upper_8_15
+@ CHECK-NEXT: movsr3, :upper8_15:_foo @ encoding: [A,0x23]
+@ CHECK-NEXT: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_thumb_upper_8_15
Index: llvm/test/MC/ARM/thumb-8-bit-relocs.s
===
--- /dev/null
+++ llvm/test/MC/ARM/thumb-8-bit-relocs.s
@@ -0,0 +1,35 @@
+@ RUN: llvm-mc -triple thumbv6m-eabi -filetype asm -o - %s | FileCheck %s
+@ RUN: llvm-mc -triple thumbv6m-eabi -filetype obj -o - %s | llvm-readobj -r - \
+@ RUN:   | FileCheck -check-prefix CHECK-RELOCATIONS %s
+@ RUN: llvm-mc -triple thumbv7m-eabi -filetype asm -o - %s | FileCheck %s
+@ RUN: llvm-mc -triple thumbv7m-eabi -filetype obj -o - %s | llvm-readobj -r - \
+@ RUN:   | FileCheck -check-prefix CHECK-RELOCATIONS %s
+
+.syntax unified
+
+.type function,%function
+function:
+  bx lr
+
+.global external
+.type external,%function
+
+.type test,%function
+test:
+  movs r3, :upper8_15:function
+  adds r3, :upper0_7:function
+  adds r3, :lower8_15:function
+  adds r3, :lower0_7:function
+
+@ CHECK-LABEL: test:
+@ CHECK:  movs r3, :upper8_15:function
+@ CHECK:  adds r3, :upper0_7:function
+@ CHECK:  adds r3, :lower8_15:function
+@ CHECK:  adds r3, :lower0_7:function
+
+@ CHECK-RELOCATIONS: Relocations [
+@ CHECK-RELOCATIONS: 0x2 R_ARM_THM_ALU_ABS_G3 function
+@ CHECK-RELOCATIONS: 0x4 R_ARM_THM_ALU_ABS_G2_NC function
+@ CHECK-RELOCATIONS: 0x6 R_ARM_THM_ALU_ABS_G1_NC function
+@ CHECK-RELOCATIONS: 0x8 R_ARM_THM_ALU_ABS_G0_NC function
+@ CHECK-RELOCATIONS: ]
Index: llvm/test/MC/ARM/negative-immediates-thumb1-fail.s
===
--- llvm/test/MC/ARM/negative-immediates-thumb1-fail.s
+++ llvm/test/MC/ARM/negative-immediates-thumb1-fail.s
@@ -12,7 +12,7 @@
 ADDs r0, #0xFEFF
 # CHECK: error: invalid instruction, any one of the following would fix this:
 # CHECK-DAG: note: invalid operand for instruction
-# CHECK-DAG: note: operand must be an immediate in the range [0,255]
+# CHECK-DAG: note: operand must be an immediate in the range [0,255] or a relocatable expression
 
 SUBs r1, r0, #0xFFF5
 # CHECK: error: invalid instruction, any one of the following would fix this:
Index: llvm/lib/Target/ARM/MCTargetDesc/ARMMCExpr.h
===
--- llvm/lib/Target/ARM/MCTargetDesc/ARMMCExpr.h
+++ llvm/lib/Target/ARM/MCTargetDesc/ARMMCExpr.h
@@ -17,8 +17,17 @@
 public:
   enum VariantKind {
 VK_ARM_None,
-VK_ARM_HI16,  // The R_ARM_MOVT_ABS relocation (:upper16: in the .s file)
-VK_ARM_LO16   // The R_ARM_MOVW_ABS_NC relocation (:lower16: in the .s file)
+VK_ARM_HI16, // The