[PATCH] D74927: [MC][ARM] make Thumb function also if type attribute is set

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 23 11:25:37 PST 2021


MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/test/MC/ARM/thumb-function-address.s:1
+@ RUN: llvm-mc -filetype=obj -triple=armv7-linux-gnueabi %s -o %t
+@ RUN: llvm-readelf -s %t | FileCheck %s
----------------
Add a file-level comment, something like:

```
@@ GNU as sets the thumb state according to the thumb state of the label.
@@ If a .type directive is placed after the label, set the symbol's thumb state according to the thumb state of the .type directive. This matches GNU as in most cases.
```
(Please reflow)

And consider adding this to the description of the patch as well. In the description, you can additionally mention that it is difficult to fully implement GNU as's rule, so we choose this heuristic.


================
Comment at: llvm/test/MC/ARM/thumb-function-address.s:27-28
+
+@@ Check func_label, foo_impl, foo_resolver, and foo addresses have bit 1 set.
+@@ Check label has bit 1 unset.
+@ CHECK:      Value Size Type Bind Vis Ndx Name
----------------
nickdesaulniers wrote:
> should this comment be `bit 0`?
Yes, `bit 0`.


================
Comment at: llvm/test/MC/ARM/thumb-function-address.s:29
+@@ Check label has bit 1 unset.
+@ CHECK:      Value Size Type Bind Vis Ndx Name
+@ CHECK-NEXT: 00000000 0 NOTYPE LOCAL DEFAULT UND
----------------
Nit: align the columns (just to make it look prettier).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74927



More information about the llvm-commits mailing list