[ARM] Support the .isnt directive for MachO and COFF targets

[mstorsjo]

Summary

Contrary to ELF, we don't add any markers that distinguish data generated with .short/.long from normal instructions, so the .inst directive only adds compatibility with assembly that uses it.

[peter.smith]

I can't comment too much on the use of .inst in machO and Windows. I have a small suggestion that might make it possible to share more of the implementation, but it is only a personal preference.

lib/Target/ARM/MCTargetDesc/ARMTargetStreamer.cpp

52 ↗

(On Diff #157764)

This looks almost identical to ARMELFStreamer::emitInst() with just the EmitARMMappingSymbol() and EmitThumbMappingSymbol().

Would it be possible to add a hook for adding the mapping symbol that the ELF backend can override and then make it use the implementation in ARMTargetStreamer?

[mstorsjo]

lib/Target/ARM/MCTargetDesc/ARMTargetStreamer.cpp

52 ↗

(On Diff #157764)

Yes, it's identical except for that, and minus some asserts about IsThumb which isn't available here.

Sharing in one way or another is doable, but it requires a little more than that. The ARMELFTargetStreamer::emitInst() method calls ARMELFStreamer::emitInst(), which contains the body of the method implementation, that we'd like to share. But ARMELFStreamer::EmitBytes() also overrides MCELFStreamer::EmitBytes to force adding a data symbol in front of it. So we can't have ARMTargetStreamer just call getStreamer().EmitBytes(StringRef(Buffer, Size)); since that will get the extra data prefix prepended.

So we can:

• Add an emitBytesCode to ARMTargetStreamer which ARMELFTargetStreamer can override, which takes care of adding the arm/thumb mapping symbol and takes care of not adding a data symbol
• Add getInstBytes to ARMTargetStreamer which both ARMTargetStreamer and ARMELFTargetStreamer can use to get the formatted bytes to print
• Something else?

[peter.smith]

lib/Target/ARM/MCTargetDesc/ARMTargetStreamer.cpp

52 ↗

(On Diff #157764)

Thanks for pointing that out; it is more complicated than I first thought. One possible alternative could be to add an overaload of emitInst with a MCObjectStreamer parameter to ARMTargetStreamer which we could call emitBytes() and potentially add the mapping symbol hooks to. The backend overrides of emitInst could add the appropriate streamer.

This is getting to the point where the extra code to factor out the duplication may not be worth it. I suggest seeing if anyone else has any preferences.

[compnerd]

@peter.smith, yeah, I think that sharing this might make it more complex. This looks fine to me, any other concerns?

[peter.smith]

In D49936#1180960, @compnerd wrote:

@peter.smith, yeah, I think that sharing this might make it more complex. This looks fine to me, any other concerns?

No further concerns. Looks good to me.

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

bool ARMAsmParser::ParseDirective(AsmToken DirectiveID) {
   else if (IDVal == ".movsp")
     parseDirectiveMovSP(DirectiveID.getLoc());
   else if (IDVal == ".arch_extension")
     parseDirectiveArchExtension(DirectiveID.getLoc());
   else if (IDVal == ".align")
     return parseDirectiveAlign(DirectiveID.getLoc()); // Use Generic on failure.
   else if (IDVal == ".thumb_set")
     parseDirectiveThumbSet(DirectiveID.getLoc());
+  else if (IDVal == ".inst")
+    parseDirectiveInst(DirectiveID.getLoc());
+  else if (IDVal == ".inst.n")
+    parseDirectiveInst(DirectiveID.getLoc(), 'n');
+  else if (IDVal == ".inst.w")
+    parseDirectiveInst(DirectiveID.getLoc(), 'w');
   else if (!IsMachO && !IsCOFF) {
     if (IDVal == ".arch")
       parseDirectiveArch(DirectiveID.getLoc());
     else if (IDVal == ".cpu")
       parseDirectiveCPU(DirectiveID.getLoc());
     else if (IDVal == ".eabi_attribute")
       parseDirectiveEabiAttr(DirectiveID.getLoc());
     else if (IDVal == ".fpu")
       parseDirectiveFPU(DirectiveID.getLoc());
     else if (IDVal == ".fnstart")
       parseDirectiveFnStart(DirectiveID.getLoc());
-    else if (IDVal == ".inst")
-      parseDirectiveInst(DirectiveID.getLoc());
-    else if (IDVal == ".inst.n")
-      parseDirectiveInst(DirectiveID.getLoc(), 'n');
-    else if (IDVal == ".inst.w")
-      parseDirectiveInst(DirectiveID.getLoc(), 'w');
     else if (IDVal == ".object_arch")
       parseDirectiveObjectArch(DirectiveID.getLoc());
     else if (IDVal == ".tlsdescseq")
       parseDirectiveTLSDescSeq(DirectiveID.getLoc());
     else
       return true;
   } else
     return true;

llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMTargetStreamer.cpp

 //===- ARMTargetStreamer.cpp - ARMTargetStreamer class --*- C++ -*---------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the ARMTargetStreamer class.
 //
 //===----------------------------------------------------------------------===//
 
 #include "ARMTargetMachine.h"
 #include "llvm/MC/ConstantPools.h"
+#include "llvm/MC/MCAsmInfo.h"
+#include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/Support/ARMBuildAttributes.h"
 #include "llvm/Support/TargetParser.h"
 
 using namespace llvm;
 

 }
 
 // finish() - write out any non-empty assembler constant pools.
 void ARMTargetStreamer::finish() { ConstantPools->emitAll(Streamer); }
 
 // reset() - Reset any state
 void ARMTargetStreamer::reset() {}
 
+void ARMTargetStreamer::emitInst(uint32_t Inst, char Suffix) {
+  unsigned Size;
+  char Buffer[4];
+  const bool LittleEndian = getStreamer().getContext().getAsmInfo()->isLittleEndian();
+
+  switch (Suffix) {
+  case '\0':
+    Size = 4;
+
+    for (unsigned II = 0, IE = Size; II != IE; II++) {
+      const unsigned I = LittleEndian ? (Size - II - 1) : II;
+      Buffer[Size - II - 1] = uint8_t(Inst >> I * CHAR_BIT);
+    }
+
+    break;
+  case 'n':
+  case 'w':
+    Size = (Suffix == 'n' ? 2 : 4);
+
+    // Thumb wide instructions are emitted as a pair of 16-bit words of the
+    // appropriate endianness.
+    for (unsigned II = 0, IE = Size; II != IE; II = II + 2) {
+      const unsigned I0 = LittleEndian ? II + 0 : II + 1;
+      const unsigned I1 = LittleEndian ? II + 1 : II + 0;
+      Buffer[Size - II - 2] = uint8_t(Inst >> I0 * CHAR_BIT);
+      Buffer[Size - II - 1] = uint8_t(Inst >> I1 * CHAR_BIT);
+    }
+
+    break;
+  default:
+    llvm_unreachable("Invalid Suffix");
+  }
+  getStreamer().EmitBytes(StringRef(Buffer, Size));
+}
+
 // The remaining callbacks should be handled separately by each
 // streamer.
 void ARMTargetStreamer::emitFnStart() {}
 void ARMTargetStreamer::emitFnEnd() {}
 void ARMTargetStreamer::emitCantUnwind() {}
 void ARMTargetStreamer::emitPersonality(const MCSymbol *Personality) {}
 void ARMTargetStreamer::emitPersonalityIndex(unsigned Index) {}
 void ARMTargetStreamer::emitHandlerData() {}

 void ARMTargetStreamer::emitIntTextAttribute(unsigned Attribute,
                                              unsigned IntValue,
                                              StringRef StringValue) {}
 void ARMTargetStreamer::emitArch(ARM::ArchKind Arch) {}
 void ARMTargetStreamer::emitArchExtension(unsigned ArchExt) {}
 void ARMTargetStreamer::emitObjectArch(ARM::ArchKind Arch) {}
 void ARMTargetStreamer::emitFPU(unsigned FPU) {}
 void ARMTargetStreamer::finishAttributeSection() {}
-void ARMTargetStreamer::emitInst(uint32_t Inst, char Suffix) {}
 void
 ARMTargetStreamer::AnnotateTLSDescriptorSequence(const MCSymbolRefExpr *SRE) {}
 void ARMTargetStreamer::emitThumbSet(MCSymbol *Symbol, const MCExpr *Value) {}
 
 static ARMBuildAttrs::CPUArch getArchForCPU(const MCSubtargetInfo &STI) {
   if (STI.getCPU() == "xscale")
     return ARMBuildAttrs::v5TEJ;
 

llvm/trunk/test/MC/ARM/directive-unsupported.s

 // CHECK: ^
 
   .eabi_attribute 0, 0
 
 // CHECK: error: unknown directive
 // CHECK: .eabi_attribute 0, 0
 // CHECK: ^
 
-  .inst 0xdefe
-
-// CHECK: error: unknown directive
-// CHECK: .inst 0xdefe
-// CHECK: ^
-
-  .inst.n 0xdefe
-
-// CHECK: error: unknown directive
-// CHECK: .inst.n 0xdefe
-// CHECK: ^
-
-  .inst.w 0xdefe
-
-// CHECK: error: unknown directive
-// CHECK: .inst.w 0xdefe
-// CHECK: ^
-
   .object_arch armv7
 
 // CHECK: error: unknown directive
 // CHECK: .object_arch armv7
 // CHECK: ^
 
   .tlsdescseq undefined
 

llvm/trunk/test/MC/ARM/inst-directive-other.s

+// RUN: llvm-mc %s -triple=armv7-apple-darwin -filetype=asm -o - \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ASM
+// RUN: llvm-mc %s -triple=armv7-apple-darwin -filetype=obj -o - \
+// RUN:   | llvm-objdump -triple=thumbv7 -d - | FileCheck %s --check-prefixes=CHECK-OBJ-CODE
+// RUN: llvm-mc %s -triple=thumbv7-win32-gnu -filetype=asm -o - \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ASM
+// RUN: llvm-mc %s -triple=thumbv7-win32-gnu -filetype=obj -o - \
+// RUN:   | llvm-objdump -d - | FileCheck %s --check-prefixes=CHECK-OBJ,CHECK-OBJ-CODE
+// RUN: llvm-mc %s -triple=armv7-linux-gnueabi -filetype=asm -o - \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ASM
+// RUN: llvm-mc %s -triple=armv7-linux-gnueabi -filetype=obj -o - \
+// RUN:   | llvm-objdump -d -triple=thumbv7 - | FileCheck %s --check-prefixes=CHECK-OBJ,CHECK-OBJ-DATA
+
+    .text
+
+    .p2align  2
+    .globl _func
+    .thumb
+_func:
+    // ELF distinguishes between data and code when emitted this way, but
+    // MachO and COFF don't.
+    bx      lr
+    .short  0x4770
+    .inst.n 0x4770
+    mov.w   r0, #42
+    .short  0xf04f, 0x002a
+    .inst.w 0xf04f002a
+
+// CHECK-ASM:        .p2align  2
+// CHECK-ASM:        .globl  _func
+// CHECK-ASM: _func:
+// CHECK-ASM:        bx      lr
+// CHECK-ASM:        .short  18288
+// CHECK-ASM:        .inst.n 0x4770
+// CHECK-ASM:        mov.w   r0, #42
+// CHECK-ASM:        .short  61519
+// CHECK-ASM:        .short  42
+// CHECK-ASM:        .inst.w 0xf04f002a
+
+// CHECK-OBJ:        0:       70 47           bx lr
+// CHECK-OBJ-CODE:   2:       70 47           bx lr
+// CHECK-OBJ-DATA:   2:       70 47           .short 0x4770
+// CHECK-OBJ:        4:       70 47           bx lr
+// CHECK-OBJ:        6:       4f f0 2a 00     mov.w   r0, #42
+// CHECK-OBJ-CODE:   a:       4f f0 2a 00     mov.w   r0, #42
+// CHECK-OBJ-DATA:   a:       4f f0 2a 00     .word 0x002af04f
+// CHECK-OBJ:        e:       4f f0 2a 00     mov.w   r0, #42