[AsmParser][ARM] Make .thumb_func imply .thumb

[LemonBoy]

Summary

GNU as documentation states that a .thumb_func directive implies .thumb, teach the asm parser to switch mode whenever it's encountered. On the other hand the labeled form, exclusive to Apple's toolchain, doesn't switch mode at all.

[LemonBoy]

llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp

11150

For reference the cctools implementation of this directive can be found here, function s_thumb_func.

[LemonBoy]

Fix LLD test.

[nickdesaulniers]

Seems to match how GAS behaves. Thanks for the patch.

[peter.smith]

LGTM too.

Could I trouble you to add a test case for something like:

        .text
        .arm
        .thumb_func
        nop
sym:
        bx lr

This just makes sure we get the nop before sym in Thumb state even though the symbol doesn't start immediately after the thumb_func.

[LemonBoy]

In D101975#2744037, @peter.smith wrote:

LGTM too.

Could I trouble you to add a test case for something like:

        .text
        .arm
        .thumb_func
        nop
sym:
        bx lr

This just makes sure we get the nop before sym in Thumb state even though the symbol doesn't start immediately after the thumb_func.

No problem, done!

lld/test/ELF/arm-ldrlit-err.s

 // REQUIRES: arm
 // RUN: llvm-mc --triple=armv7a-none-eabi --arm-add-build-attributes -filetype=obj -o %t.o %s
 // RUN: not ld.lld -n %t.o -o %t 2>&1 | FileCheck %s
  .section .text.0, "ax", %progbits
  .thumb_func
  .balign 4
 low:
   bx lr
   nop
   nop
+  nop
+  nop
+  nop
 
  .section .text.1, "ax", %progbits
  .global _start
  .arm
 _start:
 // CHECK: {{.*}}.s.tmp.o:(.text.1+0x0): relocation R_ARM_LDR_PC_G0 out of range: 4096 is not in [0, 4095]
 /// ldr r0, low - 4076
  .inst 0xe51f0ff4
  .reloc 0, R_ARM_LDR_PC_G0, low
 // CHECK: {{.*}}.s.tmp.o:(.text.1+0x4): relocation R_ARM_LDR_PC_G0 out of range: 4096 is not in [0, 4095]
 /// ldr r0, high + 4100
  .inst 0xe59f0ffc
  .reloc 4, R_ARM_LDR_PC_G0, high
 
  .section .text.2
  .thumb_func
  .balign 4
 high:
  bx lr
+ nop

llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp

 bool ARMAsmParser::parseDirectiveThumbFunc(SMLoc L) {
   MCAsmParser &Parser = getParser();
   const auto Format = getContext().getObjectFileType();
   bool IsMachO = Format == MCContext::IsMachO;
 
   // Darwin asm has (optionally) function name after .thumb_func direction
   // ELF doesn't
 
   if (IsMachO) {
     if (Parser.getTok().is(AsmToken::Identifier) ||
         Parser.getTok().is(AsmToken::String)) {
       MCSymbol *Func = getParser().getContext().getOrCreateSymbol(
           Parser.getTok().getIdentifier());
       getParser().getStreamer().emitThumbFunc(Func);
       Parser.Lex();
       if (parseToken(AsmToken::EndOfStatement,
                      "unexpected token in '.thumb_func' directive"))
         return true;
       return false;
     }
   }
 
   if (parseToken(AsmToken::EndOfStatement,
                  "unexpected token in '.thumb_func' directive"))
     return true;
 
+  // .thumb_func implies .thumb
+  if (!isThumb())
+    SwitchMode();
+
+  getParser().getStreamer().emitAssemblerFlag(MCAF_Code16);
+
   NextSymbolIsThumb = true;
   return false;
 }
 
 /// parseDirectiveSyntax
 ///  ::= .syntax unified | divided
 bool ARMAsmParser::parseDirectiveSyntax(SMLoc L) {
   MCAsmParser &Parser = getParser();

llvm/test/MC/ARM/thumb_func-implies-thumb.s

+@ RUN: llvm-mc -triple=armv7-darwin- -show-encoding < %s | FileCheck %s
+.syntax unified
+
+.text
+
+.arm
+@ Ensure the plain form switches mode.
+.thumb_func
+@ CHECK: .code 16
+@ CHECK-LABEL: foo
+foo:
+    bx lr
+
+.arm
+@ Ensure the labeled form doesn't switch mode.
+.thumb_func bar
+@ CHECK-NOT: .code 16
+@ CHECK-LABEL: bar
+bar:
+    bx lr
+
+.arm
+@ Ensure the nop is assembled in thumb mode, even though the baz symbol is
+@ defined later.
+.thumb_func
+nop
+@ CHECK: .code 16
+@ CHECK-NEXT: nop
+@ CHECK-LABEL: baz
+baz:
+    bx lr