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

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

I think this is close, but there are a couple of test failures with ninja check-llvm



================
Comment at: llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp:591
+  bool EmitSymbolAttribute(MCSymbol *S, MCSymbolAttr Attribute) {
+    bool val = MCELFStreamer::EmitSymbolAttribute(S, Attribute);
+
----------------
Yes, to both of the above. This will need to be

```
bool EmitSymbolAttribute(MCSymbol *S, MCSymbolAttr Attribute) override
```


================
Comment at: llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp:593
+
+    if (!IsThumb)
+      return val;
----------------
Sadly I don't think that this is going to work as IsThumb is the state at the point emitSymbolAttribute(S, Attribute) is called, but not necessarily the state at the declaration of S. With this change I see a couple of test failures:

```
Failing Tests (2):
    LLVM :: MC/ARM/mixed-arm-thumb-bl-fixup.ll
    LLVM :: MC/ARM/thumb2-beq-fixup.s
```
An extract from one points out the problem:

```
  .code  16
  .thumb_func
thumb_caller:
  beq.w internal_arm_fn
  beq.w global_arm_fn
  beq.w global_thumb_fn
  beq.w internal_thumb_fn

  .type  internal_arm_fn,%function
  .code  32
internal_arm_fn:
  bx  lr
```
At the point `.type internal_arm_fn, %function` is called IsThumb is true, emitSymbolAttribute will call setIsThumbFunc which is then cached. When internal_arm_fn is defined the state is Arm but the cache of ThumbFuncs remains.

I think this particular case might be fixed by only calling setIsThumbFunc when S is a definition. When internal_arm_fn is defined IsThumb will be false so we'll be ok.

It won't fix something like:

```
.thumb
corner_case_thumb:
 bx lr
.arm
some_arm_function:
.type corner_case_thumb, %function
```
I think that this is much less likely in practice, and would require the ArmBackend to track IsThumb for each symbol whether it was a function or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74927