[AArch64] Fix two bugs in the .inst directive

[olista01]

Summary

The AArch64 .inst directive was implemented using EmitIntValue, which resulted in both $x and $d (code and data) mapping symbols being emitted at the same address. This fixes it to only emit the $x mapping symbol.

EmitIntValue also emits the value in big-endian order when targeting big-endian systems, but instructions are always emitted in little-endian order for AArch64.

[rengolin]

lib/Target/AArch64/MCTargetDesc/AArch64ELFStreamer.cpp

120 ↗

(On Diff #45370)

Does this mean AArch64 BE was emitting all instructions the wrong way around until now?

[olista01]

lib/Target/AArch64/MCTargetDesc/AArch64ELFStreamer.cpp

120 ↗

(On Diff #45370)

No, this function is only used for the .inst directive.

[rengolin]

Otherwise, LGTM. Thanks!

lib/Target/AArch64/MCTargetDesc/AArch64ELFStreamer.cpp

120 ↗

(On Diff #45370)

Ah, right! EmitInstruction is just up there. ok. Can you add a one-line comment to this function saying it's *just* for .inst?

llvm/trunk/lib/Target/AArch64/MCTargetDesc/AArch64ELFStreamer.cpp

public:
   /// streamer. We override it to add the appropriate mapping symbol if
   /// necessary.
   void EmitInstruction(const MCInst &Inst,
                        const MCSubtargetInfo &STI) override {
     EmitA64MappingSymbol();
     MCELFStreamer::EmitInstruction(Inst, STI);
   }
 
+  /// Emit a 32-bit value as an instruction. This is only used for the .inst
+  /// directive, EmitInstruction should be used in other cases.
   void emitInst(uint32_t Inst) {
+    char Buffer[4];
+
+    // We can't just use EmitIntValue here, as that will emit a data mapping
+    // symbol, and swap the endianness on big-endian systems (instructions are
+    // always little-endian).
+    for (unsigned I = 0; I < 4; ++I) {
+      Buffer[I] = uint8_t(Inst);
+      Inst >>= 8;
+    }
+
     EmitA64MappingSymbol();
-    MCELFStreamer::EmitIntValue(Inst, 4);
+    MCELFStreamer::EmitBytes(StringRef(Buffer, 4));
   }
 
   /// This is one of the functions used to emit data into an ELF section, so the
   /// AArch64 streamer overrides it to add the appropriate mapping symbol ($d)
   /// if necessary.
   void EmitBytes(StringRef Data) override {
     EmitDataMappingSymbol();
     MCELFStreamer::EmitBytes(Data);

llvm/trunk/test/MC/AArch64/inst-directive.s

 // RUN: llvm-mc %s -triple=aarch64-none-linux-gnu -filetype=asm -o - \
 // RUN:   | FileCheck %s --check-prefix=CHECK-ASM
-// RUN: llvm-mc %s -triple=aarch64-none-linux-gnu -filetype=obj -o - \
-// RUN:   | llvm-readobj -s -sd | FileCheck %s  --check-prefix=CHECK-OBJ
+// RUN: llvm-mc %s -triple=aarch64-none-linux-gnu -filetype=obj -o %t
+// RUN: llvm-readobj -s -sd %t | FileCheck %s  --check-prefix=CHECK-OBJ
+// RUN: llvm-objdump -t %t | FileCheck %s  --check-prefix=CHECK-SYMS
+
+// RUN: llvm-mc %s -triple=aarch64_be-none-linux-gnu -filetype=asm -o - \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ASM
+// RUN: llvm-mc %s -triple=aarch64_be-none-linux-gnu -filetype=obj -o %t
+// RUN: llvm-readobj -s -sd %t | FileCheck %s  --check-prefix=CHECK-OBJ
+// RUN: llvm-objdump -t %t | FileCheck %s  --check-prefix=CHECK-SYMS
 
     .section    .inst.aarch64_inst
 
     .align  2
     .global aarch64_inst
     .type   aarch64_inst,%function
 aarch64_inst:
     .inst 0x5e104020
 
 // CHECK-ASM:        .align  2
 // CHECK-ASM:        .globl  aarch64_inst
 // CHECK-ASM:        .type   aarch64_inst,@function
 // CHECK-ASM: aarch64_inst:
 // CHECK-ASM:        .inst   0x5e104020
 
 // CHECK-OBJ: Section {
 // CHECK-OBJ:   Name: .inst.aarch64_inst
 // CHECK-OBJ:   SectionData (
 // CHECK-OBJ-NEXT: 0000: 2040105E
 // CHECK-OBJ-NEXT: )
+
+// CHECK-SYMS-NOT: 0000000000000000         .inst.aarch64_inst              00000000 $d
+// CHECK-SYMS:     0000000000000000         .inst.aarch64_inst              00000000 $x
+// CHECK-SYMS-NOT: 0000000000000000         .inst.aarch64_inst              00000000 $d