[AMDGPU][RFC] Combine asm and disasm tests. #90214

kosarev · 2024-04-26T13:53:30Z

Eliminates the need to replicate the same instructions in MC and MC/Disassembler tests and synchronize changes in them. Also highlights differences between disassembled, reassembled and original instructions.

llvmbot · 2024-04-26T13:54:02Z

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-amdgpu

Author: Ivan Kosarev (kosarev)

Changes

Eliminates the need to replicate the same instructions in MC and MC/Disassembler tests and synchronize changes in them. Also highlights differences between disassembled, reassembled and original instructions.

Patch is 316.37 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/90214.diff

6 Files Affected:

(modified) llvm/include/llvm/MC/TargetRegistry.h (+37-1)
(modified) llvm/lib/MC/CMakeLists.txt (+1)
(added) llvm/lib/MC/MCHexStreamer.cpp (+89)
(modified) llvm/test/MC/AMDGPU/gfx12_asm_vop1.s (+1205-1198)
(removed) llvm/test/MC/Disassembler/AMDGPU/gfx12_dasm_vop1.txt (-3338)
(modified) llvm/tools/llvm-mc/llvm-mc.cpp (+24-20)

diff --git a/llvm/include/llvm/MC/TargetRegistry.h b/llvm/include/llvm/MC/TargetRegistry.h
index 47051447404d00..bc6ceb7c9f5d66 100644
--- a/llvm/include/llvm/MC/TargetRegistry.h
+++ b/llvm/include/llvm/MC/TargetRegistry.h
@@ -88,7 +88,11 @@ createAsmStreamer(MCContext &Ctx, std::unique_ptr<formatted_raw_ostream> OS,
                   bool isVerboseAsm, bool useDwarfDirectory,
                   MCInstPrinter *InstPrint, std::unique_ptr<MCCodeEmitter> &&CE,
                   std::unique_ptr<MCAsmBackend> &&TAB, bool ShowInst);
-
+MCStreamer *createHexStreamer(MCContext &Ctx,
+                              std::unique_ptr<formatted_raw_ostream> OS,
+                              MCInstPrinter *InstPrint,
+                              std::unique_ptr<MCCodeEmitter> &&CE,
+                              std::unique_ptr<MCAsmBackend> &&TAB);
 MCStreamer *createELFStreamer(MCContext &Ctx,
                               std::unique_ptr<MCAsmBackend> &&TAB,
                               std::unique_ptr<MCObjectWriter> &&OW,
@@ -240,6 +244,9 @@ class Target {
   using AsmTargetStreamerCtorTy = MCTargetStreamer *(*)(
       MCStreamer &S, formatted_raw_ostream &OS, MCInstPrinter *InstPrint,
       bool IsVerboseAsm);
+  using HexTargetStreamerCtorTy =
+      MCTargetStreamer *(*)(MCStreamer &S, formatted_raw_ostream &OS,
+                            MCInstPrinter *InstPrint);
   using ObjectTargetStreamerCtorTy = MCTargetStreamer *(*)(
       MCStreamer &S, const MCSubtargetInfo &STI);
   using MCRelocationInfoCtorTy = MCRelocationInfo *(*)(const Triple &TT,
@@ -352,6 +359,10 @@ class Target {
   /// registered (default = nullptr).
   AsmTargetStreamerCtorTy AsmTargetStreamerCtorFn = nullptr;
 
+  /// Construction function for this target's hex TargetStreamer, if
+  /// registered (default = nullptr).
+  HexTargetStreamerCtorTy HexTargetStreamerCtorFn = nullptr;
+
   /// Construction function for this target's obj TargetStreamer, if
   /// registered (default = nullptr).
   ObjectTargetStreamerCtorTy ObjectTargetStreamerCtorFn = nullptr;
@@ -680,6 +691,26 @@ class Target {
     return nullptr;
   }
 
+  MCStreamer *createHexStreamer(MCContext &Ctx,
+                                std::unique_ptr<formatted_raw_ostream> OS,
+                                MCInstPrinter *InstPrint,
+                                std::unique_ptr<MCCodeEmitter> &&CE,
+                                std::unique_ptr<MCAsmBackend> &&TAB) const {
+    formatted_raw_ostream &OSRef = *OS;
+    MCStreamer *S = llvm::createHexStreamer(Ctx, std::move(OS), InstPrint,
+                                            std::move(CE), std::move(TAB));
+    createHexTargetStreamer(*S, OSRef, InstPrint);
+    return S;
+  }
+
+  MCTargetStreamer *createHexTargetStreamer(MCStreamer &S,
+                                            formatted_raw_ostream &OS,
+                                            MCInstPrinter *InstPrint) const {
+    if (HexTargetStreamerCtorFn)
+      return HexTargetStreamerCtorFn(S, OS, InstPrint);
+    return nullptr;
+  }
+
   /// createMCRelocationInfo - Create a target specific MCRelocationInfo.
   ///
   /// \param TT The target triple.
@@ -1058,6 +1089,11 @@ struct TargetRegistry {
     T.AsmTargetStreamerCtorFn = Fn;
   }
 
+  static void RegisterHexTargetStreamer(Target &T,
+                                        Target::HexTargetStreamerCtorTy Fn) {
+    T.HexTargetStreamerCtorFn = Fn;
+  }
+
   static void
   RegisterObjectTargetStreamer(Target &T,
                                Target::ObjectTargetStreamerCtorTy Fn) {
diff --git a/llvm/lib/MC/CMakeLists.txt b/llvm/lib/MC/CMakeLists.txt
index a089d2bff94f42..55bde593fed3c6 100644
--- a/llvm/lib/MC/CMakeLists.txt
+++ b/llvm/lib/MC/CMakeLists.txt
@@ -25,6 +25,7 @@ add_llvm_component_library(LLVMMC
   MCExpr.cpp
   MCFragment.cpp
   MCGOFFStreamer.cpp
+  MCHexStreamer.cpp
   MCInst.cpp
   MCInstPrinter.cpp
   MCInstrAnalysis.cpp
diff --git a/llvm/lib/MC/MCHexStreamer.cpp b/llvm/lib/MC/MCHexStreamer.cpp
new file mode 100644
index 00000000000000..b568729d132fd8
--- /dev/null
+++ b/llvm/lib/MC/MCHexStreamer.cpp
@@ -0,0 +1,89 @@
+//===- lib/MC/MCHexStreamer.cpp - Hex Output --------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/MC/MCAsmBackend.h"
+#include "llvm/MC/MCAssembler.h"
+#include "llvm/MC/MCCodeEmitter.h"
+#include "llvm/MC/MCInstPrinter.h"
+#include "llvm/MC/MCObjectWriter.h"
+#include "llvm/MC/MCStreamer.h"
+#include "llvm/MC/TargetRegistry.h"
+
+using namespace llvm;
+
+namespace {
+
+class MCHexStreamer final : public MCStreamer {
+  std::unique_ptr<formatted_raw_ostream> OSOwner;
+  formatted_raw_ostream &OS;
+  std::unique_ptr<MCInstPrinter> InstPrinter;
+  std::unique_ptr<MCAssembler> Assembler;
+
+  raw_null_ostream NullStream;
+
+public:
+  MCHexStreamer(MCContext &Context, std::unique_ptr<formatted_raw_ostream> os,
+                MCInstPrinter *printer, std::unique_ptr<MCCodeEmitter> emitter,
+                std::unique_ptr<MCAsmBackend> asmbackend)
+      : MCStreamer(Context), OSOwner(std::move(os)), OS(*OSOwner),
+        InstPrinter(printer),
+        Assembler(std::make_unique<MCAssembler>(
+            Context, std::move(asmbackend), std::move(emitter),
+            asmbackend ? asmbackend->createObjectWriter(NullStream)
+                       : nullptr)) {
+    assert(InstPrinter);
+  }
+
+  MCAssembler &getAssembler() { return *Assembler; }
+
+  /// @name MCStreamer Interface
+  /// @{
+
+  bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override {
+    return true;
+  }
+
+  void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
+                        Align ByteAlignment) override {}
+
+  void emitZerofill(MCSection *Section, MCSymbol *Symbol = nullptr,
+                    uint64_t Size = 0, Align ByteAlignment = Align(1),
+                    SMLoc Loc = SMLoc()) override {}
+
+  void emitInstruction(const MCInst &Inst, const MCSubtargetInfo &STI) override;
+};
+
+} // end anonymous namespace.
+
+void MCHexStreamer::emitInstruction(const MCInst &Inst,
+                                    const MCSubtargetInfo &STI) {
+  SmallString<256> Code;
+  SmallVector<MCFixup, 4> Fixups;
+  getAssembler().getEmitter().encodeInstruction(Inst, Code, Fixups, STI);
+
+  OS << "[";
+  for (auto [I, B] : enumerate(Code))
+    OS << (I != 0 ? "," : "") << format("0x%02x", B & 0xff);
+  OS << "]  #";
+
+  if (getTargetStreamer())
+    getTargetStreamer()->prettyPrintAsm(*InstPrinter, 0, Inst, STI, OS);
+  else
+    InstPrinter->printInst(&Inst, 0, "", STI, OS);
+
+  OS << "\n";
+}
+
+MCStreamer *llvm::createHexStreamer(MCContext &Context,
+                                    std::unique_ptr<formatted_raw_ostream> OS,
+                                    MCInstPrinter *IP,
+                                    std::unique_ptr<MCCodeEmitter> &&CE,
+                                    std::unique_ptr<MCAsmBackend> &&MAB) {
+  return new MCHexStreamer(Context, std::move(OS), IP, std::move(CE),
+                           std::move(MAB));
+}
diff --git a/llvm/test/MC/AMDGPU/gfx12_asm_vop1.s b/llvm/test/MC/AMDGPU/gfx12_asm_vop1.s
index 5279588f050629..172d3ea5157caa 100644
--- a/llvm/test/MC/AMDGPU/gfx12_asm_vop1.s
+++ b/llvm/test/MC/AMDGPU/gfx12_asm_vop1.s
@@ -1,3590 +1,3597 @@
-// RUN: llvm-mc -triple=amdgcn -mcpu=gfx1200 -mattr=+wavefrontsize32,-wavefrontsize64 -show-encoding %s | FileCheck --check-prefix=GFX12 %s
-// RUN: llvm-mc -triple=amdgcn -mcpu=gfx1200 -mattr=-wavefrontsize32,+wavefrontsize64 -show-encoding %s | FileCheck --check-prefix=GFX12 %s
+// RUN: llvm-mc -triple=amdgcn -mcpu=gfx1200 -mattr=+wavefrontsize32,-wavefrontsize64 -show-encoding %s | FileCheck --strict-whitespace --check-prefixes=GFX12,GFX12-ASM %s
+// RUN: llvm-mc -triple=amdgcn -mcpu=gfx1200 -mattr=+wavefrontsize32,-wavefrontsize64 -show-encoding -filetype=hex %s | llvm-mc -triple=amdgcn -mcpu=gfx1200 -mattr=+wavefrontsize32,-wavefrontsize64 -disassemble -show-encoding | FileCheck --strict-whitespace --check-prefixes=GFX12,GFX12-DIS %s
+// RUN: llvm-mc -triple=amdgcn -mcpu=gfx1200 -mattr=-wavefrontsize32,+wavefrontsize64 -show-encoding %s | FileCheck --strict-whitespace --check-prefixes=GFX12,GFX12-ASM %s
+// RUN: llvm-mc -triple=amdgcn -mcpu=gfx1200 -mattr=-wavefrontsize32,+wavefrontsize64 -show-encoding -filetype=hex %s | llvm-mc -triple=amdgcn -mcpu=gfx1200 -mattr=-wavefrontsize32,+wavefrontsize64 -disassemble -show-encoding | FileCheck --strict-whitespace --check-prefixes=GFX12,GFX12-DIS %s
 
 v_bfrev_b32_e32 v5, v1
-// GFX12: encoding: [0x01,0x71,0x0a,0x7e]
+// GFX12: v_bfrev_b32_e32 v5, v1                  ; encoding: [0x01,0x71,0x0a,0x7e]
 
 v_bfrev_b32 v5, v255
-// GFX12: encoding: [0xff,0x71,0x0a,0x7e]
+// GFX12: v_bfrev_b32_e32 v5, v255                ; encoding: [0xff,0x71,0x0a,0x7e]
 
 v_bfrev_b32 v5, s1
-// GFX12: encoding: [0x01,0x70,0x0a,0x7e]
+// GFX12: v_bfrev_b32_e32 v5, s1                  ; encoding: [0x01,0x70,0x0a,0x7e]
 
 v_bfrev_b32 v5, s105
-// GFX12: encoding: [0x69,0x70,0x0a,0x7e]
+// GFX12: v_bfrev_b32_e32 v5, s105                ; encoding: [0x69,0x70,0x0a,0x7e]
 
 v_bfrev_b32 v5, vcc_lo
-// GFX12: encoding: [0x6a,0x70,0x0a,0x7e]
+// GFX12: v_bfrev_b32_e32 v5, vcc_lo              ; encoding: [0x6a,0x70,0x0a,0x7e]
 
 v_bfrev_b32 v5, vcc_hi
-// GFX12: encoding: [0x6b,0x70,0x0a,0x7e]
+// GFX12: v_bfrev_b32_e32 v5, vcc_hi              ; encoding: [0x6b,0x70,0x0a,0x7e]
 
 v_bfrev_b32 v5, ttmp15
-// GFX12: encoding: [0x7b,0x70,0x0a,0x7e]
+// GFX12: v_bfrev_b32_e32 v5, ttmp15              ; encoding: [0x7b,0x70,0x0a,0x7e]
 
 v_bfrev_b32 v5, m0
-// GFX12: encoding: [0x7d,0x70,0x0a,0x7e]
+// GFX12: v_bfrev_b32_e32 v5, m0                  ; encoding: [0x7d,0x70,0x0a,0x7e]
 
 v_bfrev_b32 v5, exec_lo
-// GFX12: encoding: [0x7e,0x70,0x0a,0x7e]
+// GFX12: v_bfrev_b32_e32 v5, exec_lo             ; encoding: [0x7e,0x70,0x0a,0x7e]
 
 v_bfrev_b32 v5, exec_hi
-// GFX12: encoding: [0x7f,0x70,0x0a,0x7e]
+// GFX12: v_bfrev_b32_e32 v5, exec_hi             ; encoding: [0x7f,0x70,0x0a,0x7e]
 
 v_bfrev_b32 v5, null
-// GFX12: encoding: [0x7c,0x70,0x0a,0x7e]
+// GFX12: v_bfrev_b32_e32 v5, null                ; encoding: [0x7c,0x70,0x0a,0x7e]
 
 v_bfrev_b32 v5, -1
-// GFX12: encoding: [0xc1,0x70,0x0a,0x7e]
+// GFX12: v_bfrev_b32_e32 v5, -1                  ; encoding: [0xc1,0x70,0x0a,0x7e]
 
 v_bfrev_b32 v5, 0.5
-// GFX12: encoding: [0xf0,0x70,0x0a,0x7e]
+// GFX12: v_bfrev_b32_e32 v5, 0.5                 ; encoding: [0xf0,0x70,0x0a,0x7e]
 
 v_bfrev_b32 v5, src_scc
-// GFX12: encoding: [0xfd,0x70,0x0a,0x7e]
+// GFX12: v_bfrev_b32_e32 v5, src_scc             ; encoding: [0xfd,0x70,0x0a,0x7e]
 
 v_bfrev_b32 v255, 0xaf123456
-// GFX12: encoding: [0xff,0x70,0xfe,0x7f,0x56,0x34,0x12,0xaf]
+// GFX12: v_bfrev_b32_e32 v255, 0xaf123456        ; encoding: [0xff,0x70,0xfe,0x7f,0x56,0x34,0x12,0xaf]
 
 v_ceil_f16 v5, v1
-// GFX12: encoding: [0x01,0xb9,0x0a,0x7e]
+// GFX12: v_ceil_f16_e32 v5, v1                   ; encoding: [0x01,0xb9,0x0a,0x7e]
 
 v_ceil_f16 v5, v127
-// GFX12: encoding: [0x7f,0xb9,0x0a,0x7e]
+// GFX12: v_ceil_f16_e32 v5, v127                 ; encoding: [0x7f,0xb9,0x0a,0x7e]
 
 v_ceil_f16 v5, s1
-// GFX12: encoding: [0x01,0xb8,0x0a,0x7e]
+// GFX12: v_ceil_f16_e32 v5, s1                   ; encoding: [0x01,0xb8,0x0a,0x7e]
 
 v_ceil_f16 v5, s105
-// GFX12: encoding: [0x69,0xb8,0x0a,0x7e]
+// GFX12: v_ceil_f16_e32 v5, s105                 ; encoding: [0x69,0xb8,0x0a,0x7e]
 
 v_ceil_f16 v5, vcc_lo
-// GFX12: encoding: [0x6a,0xb8,0x0a,0x7e]
+// GFX12: v_ceil_f16_e32 v5, vcc_lo               ; encoding: [0x6a,0xb8,0x0a,0x7e]
 
 v_ceil_f16 v5, vcc_hi
-// GFX12: encoding: [0x6b,0xb8,0x0a,0x7e]
+// GFX12: v_ceil_f16_e32 v5, vcc_hi               ; encoding: [0x6b,0xb8,0x0a,0x7e]
 
 v_ceil_f16 v5, ttmp15
-// GFX12: encoding: [0x7b,0xb8,0x0a,0x7e]
+// GFX12: v_ceil_f16_e32 v5, ttmp15               ; encoding: [0x7b,0xb8,0x0a,0x7e]
 
 v_ceil_f16 v5, m0
-// GFX12: encoding: [0x7d,0xb8,0x0a,0x7e]
+// GFX12: v_ceil_f16_e32 v5, m0                   ; encoding: [0x7d,0xb8,0x0a,0x7e]
 
 v_ceil_f16 v5, exec_lo
-// GFX12: encoding: [0x7e,0xb8,0x0a,0x7e]
+// GFX12: v_ceil_f16_e32 v5, exec_lo              ; encoding: [0x7e,0xb8,0x0a,0x7e]
 
 v_ceil_f16 v5, exec_hi
-// GFX12: encoding: [0x7f,0xb8,0x0a,0x7e]
+// GFX12: v_ceil_f16_e32 v5, exec_hi              ; encoding: [0x7f,0xb8,0x0a,0x7e]
 
 v_ceil_f16 v5, null
-// GFX12: encoding: [0x7c,0xb8,0x0a,0x7e]
+// GFX12: v_ceil_f16_e32 v5, null                 ; encoding: [0x7c,0xb8,0x0a,0x7e]
 
 v_ceil_f16 v5, -1
-// GFX12: encoding: [0xc1,0xb8,0x0a,0x7e]
+// GFX12: v_ceil_f16_e32 v5, -1                   ; encoding: [0xc1,0xb8,0x0a,0x7e]
 
 v_ceil_f16 v5, 0.5
-// GFX12: encoding: [0xf0,0xb8,0x0a,0x7e]
+// GFX12: v_ceil_f16_e32 v5, 0.5                  ; encoding: [0xf0,0xb8,0x0a,0x7e]
 
 v_ceil_f16 v5, src_scc
-// GFX12: encoding: [0xfd,0xb8,0x0a,0x7e]
+// GFX12: v_ceil_f16_e32 v5, src_scc              ; encoding: [0xfd,0xb8,0x0a,0x7e]
 
 v_ceil_f16 v127, 0xfe0b
-// GFX12: encoding: [0xff,0xb8,0xfe,0x7e,0x0b,0xfe,0x00,0x00]
+// GFX12: v_ceil_f16_e32 v127, 0xfe0b             ; encoding: [0xff,0xb8,0xfe,0x7e,0x0b,0xfe,0x00,0x00]
 
 v_ceil_f32 v5, v1
-// GFX12: encoding: [0x01,0x45,0x0a,0x7e]
+// GFX12: v_ceil_f32_e32 v5, v1                   ; encoding: [0x01,0x45,0x0a,0x7e]
 
 v_ceil_f32 v5, v255
-// GFX12: encoding: [0xff,0x45,0x0a,0x7e]
+// GFX12: v_ceil_f32_e32 v5, v255                 ; encoding: [0xff,0x45,0x0a,0x7e]
 
 v_ceil_f32 v5, s1
-// GFX12: encoding: [0x01,0x44,0x0a,0x7e]
+// GFX12: v_ceil_f32_e32 v5, s1                   ; encoding: [0x01,0x44,0x0a,0x7e]
 
 v_ceil_f32 v5, s105
-// GFX12: encoding: [0x69,0x44,0x0a,0x7e]
+// GFX12: v_ceil_f32_e32 v5, s105                 ; encoding: [0x69,0x44,0x0a,0x7e]
 
 v_ceil_f32 v5, vcc_lo
-// GFX12: encoding: [0x6a,0x44,0x0a,0x7e]
+// GFX12: v_ceil_f32_e32 v5, vcc_lo               ; encoding: [0x6a,0x44,0x0a,0x7e]
 
 v_ceil_f32 v5, vcc_hi
-// GFX12: encoding: [0x6b,0x44,0x0a,0x7e]
+// GFX12: v_ceil_f32_e32 v5, vcc_hi               ; encoding: [0x6b,0x44,0x0a,0x7e]
 
 v_ceil_f32 v5, ttmp15
-// GFX12: encoding: [0x7b,0x44,0x0a,0x7e]
+// GFX12: v_ceil_f32_e32 v5, ttmp15               ; encoding: [0x7b,0x44,0x0a,0x7e]
 
 v_ceil_f32 v5, m0
-// GFX12: encoding: [0x7d,0x44,0x0a,0x7e]
+// GFX12: v_ceil_f32_e32 v5, m0                   ; encoding: [0x7d,0x44,0x0a,0x7e]
 
 v_ceil_f32 v5, exec_lo
-// GFX12: encoding: [0x7e,0x44,0x0a,0x7e]
+// GFX12: v_ceil_f32_e32 v5, exec_lo              ; encoding: [0x7e,0x44,0x0a,0x7e]
 
 v_ceil_f32 v5, exec_hi
-// GFX12: encoding: [0x7f,0x44,0x0a,0x7e]
+// GFX12: v_ceil_f32_e32 v5, exec_hi              ; encoding: [0x7f,0x44,0x0a,0x7e]
 
 v_ceil_f32 v5, null
-// GFX12: encoding: [0x7c,0x44,0x0a,0x7e]
+// GFX12: v_ceil_f32_e32 v5, null                 ; encoding: [0x7c,0x44,0x0a,0x7e]
 
 v_ceil_f32 v5, -1
-// GFX12: encoding: [0xc1,0x44,0x0a,0x7e]
+// GFX12: v_ceil_f32_e32 v5, -1                   ; encoding: [0xc1,0x44,0x0a,0x7e]
 
 v_ceil_f32 v5, 0.5
-// GFX12: encoding: [0xf0,0x44,0x0a,0x7e]
+// GFX12: v_ceil_f32_e32 v5, 0.5                  ; encoding: [0xf0,0x44,0x0a,0x7e]
 
 v_ceil_f32 v5, src_scc
-// GFX12: encoding: [0xfd,0x44,0x0a,0x7e]
+// GFX12: v_ceil_f32_e32 v5, src_scc              ; encoding: [0xfd,0x44,0x0a,0x7e]
 
 v_ceil_f32 v255, 0xaf123456
-// GFX12: encoding: [0xff,0x44,0xfe,0x7f,0x56,0x34,0x12,0xaf]
+// GFX12: v_ceil_f32_e32 v255, 0xaf123456         ; encoding: [0xff,0x44,0xfe,0x7f,0x56,0x34,0x12,0xaf]
 
 v_ceil_f64 v[5:6], v[1:2]
-// GFX12: encoding: [0x01,0x31,0x0a,0x7e]
+// GFX12: v_ceil_f64_e32 v[5:6], v[1:2]           ; encoding: [0x01,0x31,0x0a,0x7e]
 
 v_ceil_f64 v[5:6], v[254:255]
-// GFX12: encoding: [0xfe,0x31,0x0a,0x7e]
+// GFX12: v_ceil_f64_e32 v[5:6], v[254:255]       ; encoding: [0xfe,0x31,0x0a,0x7e]
 
 v_ceil_f64 v[5:6], s[2:3]
-// GFX12: encoding: [0x02,0x30,0x0a,0x7e]
+// GFX12: v_ceil_f64_e32 v[5:6], s[2:3]           ; encoding: [0x02,0x30,0x0a,0x7e]
 
 v_ceil_f64 v[5:6], s[104:105]
-// GFX12: encoding: [0x68,0x30,0x0a,0x7e]
+// GFX12: v_ceil_f64_e32 v[5:6], s[104:105]       ; encoding: [0x68,0x30,0x0a,0x7e]
 
 v_ceil_f64 v[5:6], vcc
-// GFX12: encoding: [0x6a,0x30,0x0a,0x7e]
+// GFX12: v_ceil_f64_e32 v[5:6], vcc              ; encoding: [0x6a,0x30,0x0a,0x7e]
 
 v_ceil_f64 v[5:6], ttmp[14:15]
-// GFX12: encoding: [0x7a,0x30,0x0a,0x7e]
+// GFX12: v_ceil_f64_e32 v[5:6], ttmp[14:15]      ; encoding: [0x7a,0x30,0x0a,0x7e]
 
 v_ceil_f64 v[5:6], exec
-// GFX12: encoding: [0x7e,0x30,0x0a,0x7e]
+// GFX12: v_ceil_f64_e32 v[5:6], exec             ; encoding: [0x7e,0x30,0x0a,0x7e]
 
 v_ceil_f64 v[5:6], null
-// GFX12: encoding: [0x7c,0x30,0x0a,0x7e]
+// GFX12: v_ceil_f64_e32 v[5:6], null             ; encoding: [0x7c,0x30,0x0a,0x7e]
 
 v_ceil_f64 v[5:6], -1
-// GFX12: encoding: [0xc1,0x30,0x0a,0x7e]
+// GFX12: v_ceil_f64_e32 v[5:6], -1               ; encoding: [0xc1,0x30,0x0a,0x7e]
 
 v_ceil_f64 v[5:6], 0.5
-// GFX12: encoding: [0xf0,0x30,0x0a,0x7e]
+// GFX12: v_ceil_f64_e32 v[5:6], 0.5              ; encoding: [0xf0,0x30,0x0a,0x7e]
 
 v_ceil_f64 v[5:6], src_scc
-// GFX12: encoding: [0xfd,0x30,0x0a,0x7e]
+// GFX12: v_ceil_f64_e32 v[5:6], src_scc          ; encoding: [0xfd,0x30,0x0a,0x7e]
 
 v_ceil_f64 v[254:255], 0xaf123456
-// GFX12: encoding: [0xff,0x30,0xfc,0x7f,0x56,0x34,0x12,0xaf]
+// GFX12: v_ceil_f64_e32 v[254:255], 0xaf123456   ; encoding: [0xff,0x30,0xfc,0x7f,0x56,0x34,0x12,0xaf]
 
 v_cls_i32 v5, v1
-// GFX12: encoding: [0x01,0x77,0x0a,0x7e]
+// GFX12: v_cls_i32_e32 v5, v1                    ; encoding: [0x01,0x77,0x0a,0x7e]
 
 v_cls_i32 v5, v255
-// GFX12: encoding: [0xff,0x77,0x0a,0x7e]
+// GFX12: v_cls_i32_e32 v5, v255                  ; encoding: [0xff,0x77,0x0a,0x7e]
 
 v_cls_i32 v5, s1
-// GFX12: encoding: [0x01,0x76,0x0a,0x7e]
+// GFX12: v_cls_i32_e32 v5, s1                    ; encoding: [0x01,0x76,0x0a,0x7e]
 
 v_cls_i32 v5, s105
-// GFX12: encoding: [0x69,0x76,0x0a,0x7e]
+// GFX12: v_cls_i32_e32 v5, s105                  ; encoding: [0x69,0x76,0x0a,0x7e]
 
 v_cls_i32 v5, vcc_lo
-// GFX12: encoding: [0x6a,0x76,0x0a,0x7e]
+// GFX12: v_cls_i32_e32 v5, vcc_lo                ; encoding: [0x6a,0x76,0x0a,0x7e]
 
 v_cls_i32 v5, vcc_hi
-// GFX12: encoding: [0x6b,0x76,0x0a,0x7e]
+// GFX12: v_cls_i32_e32 v5, vcc_hi                ; encoding: [0x6b,0x76,0x0a,0x7e]
 
 v_cls_i32 v5, ttmp15
-// GFX12: encoding: [0x7b,0x76,0x0a,0x7e]
+// GFX12: v_cls_i32_e32 v5, ttmp15                ; encoding: [0x7b,0x76,0x0a,0x7e]
 
 v_cls_i32 v5, m0
-// GFX12: encoding: [0x7d,0x76,0x0a,0x7e]
+// GFX12: v_cls_i32_e32 v5, m0                    ; encoding: [0x7d,0x76,0x0a,0x7e]
 
 v_cls_i32 v5, exec_lo
-// GFX12: encoding: [0x7e,0x76,0x0a,0x7e]
+// GFX12: v_cls_i32_e32 v5, exec_lo               ; encoding: [0x7e,0x76,0x0a,0x7e]
 
 v_cls_i32 v5, exec_hi
-// GFX12: encoding: [0x7f,0x76,0x0a,0x7e]
+// GFX12: v_cls_i32_e32 v5, exec_hi               ; encoding: [0x7f,0x76,0x0a,0x7e]
 
 v_cls_i32 v5, null
-// GFX12: encoding: [0x7c,0x76,0x0a,0x7e]
+// GFX12: v_cls_i32_e32 v5, null                  ; encoding: [0x7c,0x76,0x0a,0x7e]
 
 v_cls_i32 v5, -1
-// GFX12: encoding: [0xc1,0x76,0x0a,0x7e]
+// GFX12: v_cls_i32_e32 v5, -1                    ; encoding: [0xc1,0x76,0x0a,0x7e]
 
 v_cls_i32 v5, 0.5
-// GFX12: encoding: [0xf0,0x76,0x0a,0x7e]
+// GFX12: v_cls_i32_e32 v5, 0.5                   ; encoding: [0xf0,0x76,0x0a,0x7e]
 
 v_cls_i32 v5, src_scc
-// GFX12: encoding: [0xfd,0x76,0x0a,0x7e]
+// GFX12: v_cls_i32_e32 v5, src_scc               ; encoding: [0xfd,0x76,0x0a,0x7e]
 
 v_cls_i32 v255, 0xaf123456
-// GFX12: encoding: [0xff,0x76,0xfe,0x7f,0x56,0x34,0x12,0xaf]
+// GFX12: v_cls_i32_e32 v255, 0xaf123456          ; encoding: [0xff,0x76,0xfe,0x7f,0x56,0x34,0x12,0xaf]
 
 v_clz_i32_u32 v5, v1
-// GFX12: encoding: [0x01,0x73,0x0a,0x7e]
+// GFX12: v_clz_i32_u32_e32 v5, v1                ; encoding: [0x01,0x73,0x0a,0x7e]
 
 v_clz_i32_u32 v5, v255
-// GFX12: encoding: [0xff,0x73,0x0a,0x7e]
+// GFX12: v_clz_i32_u32_e32 v5, v255              ; encoding: [0xff,0x73,0x0a,0x7e]
 
 v_clz_i32_u32 v5, s1
-// GFX12: encoding: [0x01,0x72,0x0a,0x7e]
+// GFX12: v_clz_i32_u32_e32 v5, s1                ; encoding: [0x01,0x72,0x0a,0x7e]
 
 v_clz_i32_u32 v5, ...
[truncated]

MaskRay · 2024-04-26T18:17:18Z

I hope that we do not add new MCStreamer derivatives just for testing purposes.
For round tripping tests, can you do llvm-mc ... -filetype=obj | llvm-objdump -d - instead?

MaskRay

.

kosarev · 2024-04-30T10:04:40Z

@MaskRay llvm-objdump -d doesn't use the asm streamer, meaning it would require separate check lines, whereas the idea is to make same check lines work for both the assembler and disassembler tests.

Regarding round-tripping, we may want to consider that at some point, but we still need to directly check what the disassembler produces, because reassembling may mask problems.

MaskRay · 2024-05-01T04:09:37Z

@MaskRay llvm-objdump -d doesn't use the asm streamer, meaning it would require separate check lines, whereas the idea is to make same check lines work for both the assembler and disassembler tests.

Regarding round-tripping, we may want to consider that at some point, but we still need to directly check what the disassembler produces, because reassembling may mask problems.

Can you give an example that llvm-objdump is insufficient for your case?

kosarev · 2024-05-01T08:59:59Z

Can you give an example that llvm-objdump is insufficient for your case?

Using llvm-objdump would require having two separate check lines for every instruction, avoiding which is the point of the patch.

v_bfrev_b32_e32 v5, v1
// GFX12-ASM: v_bfrev_b32_e32 v5, v1                  ; encoding: [0x01,0x71,0x0a,0x7e]
// GFX12-DIS: v_bfrev_b32_e32 v5, v1                                     // 000000000000: 7E0A7101

MaskRay · 2024-05-01T17:36:02Z

Can you give an example that llvm-objdump is insufficient for your case?

Using llvm-objdump would require having two separate check lines for every instruction, avoiding which is the point of the patch.
v_bfrev_b32_e32 v5, v1
// GFX12-ASM: v_bfrev_b32_e32 v5, v1                  ; encoding: [0x01,0x71,0x0a,0x7e]
// GFX12-DIS: v_bfrev_b32_e32 v5, v1                                     // 000000000000: 7E0A7101

Utilize -SAME: directives?

# RUN: llvm-mc ... %s | FileCheck
# RUN: llvm-mc -filetype=obj ... %s | llvm-objdump -d - | FileCheck --check-prefixes=CHECK,ENCODING

# CHECK: nop
# ENCODING-SAME: [0,0,0,0]
nop

kosarev · 2024-05-01T23:20:37Z

Utilize -SAME: directives?

# RUN: llvm-mc ... %s | FileCheck
# RUN: llvm-mc -filetype=obj ... %s | llvm-objdump -d - | FileCheck --check-prefixes=CHECK,ENCODING

# CHECK: nop
# ENCODING-SAME: [0,0,0,0]
nop

Again, this wouldn't work because llvm-objdump -d does not print encodings in the [0,0,0,0] form.

kosarev · 2024-05-01T23:24:48Z

Also, the first RUN line doesn't check the resulting instruction code, so doesn't work as an assembler-side test.

MaskRay · 2024-05-03T07:24:59Z

Utilize -SAME: directives?
# RUN: llvm-mc ... %s | FileCheck
# RUN: llvm-mc -filetype=obj ... %s | llvm-objdump -d - | FileCheck --check-prefixes=CHECK,ENCODING

# CHECK: nop
# ENCODING-SAME: [0,0,0,0]
nop
Again, this wouldn't work because llvm-objdump -d does not print encodings in the [0,0,0,0] form.

llvm-objdump seems to places the encodings as comments. You can check them with -SAME:.

Also, the first RUN line doesn't check the resulting instruction code, so doesn't work as an assembler-side test.

I am confused. Why is llvm-mc -filetype=obj ... %s | llvm-objdump -d - not checking the result?

arsenm · 2024-05-08T07:44:50Z

llvm/lib/MC/MCHexStreamer.cpp

+
+  OS << "[";
+  for (auto [I, B] : enumerate(Code))
+    OS << (I != 0 ? "," : "") << format("0x%02x", B & 0xff);


Use ListSeparator for the =0 case

arsenm · 2024-05-08T07:44:56Z

llvm/lib/MC/MCHexStreamer.cpp

+  SmallVector<MCFixup, 4> Fixups;
+  getAssembler().getEmitter().encodeInstruction(Inst, Code, Fixups, STI);
+
+  OS << "[";


Suggested change

OS << "[";

OS << '[';

arsenm · 2024-05-08T07:45:04Z

llvm/lib/MC/MCHexStreamer.cpp

+  else
+    InstPrinter->printInst(&Inst, 0, "", STI, OS);
+
+  OS << "\n";


Suggested change

OS << "\n";

OS << '\n';

arsenm · 2024-05-08T07:45:33Z

llvm/tools/llvm-mc/llvm-mc.cpp

+                        Ctx, std::move(FOut), /*asmverbose*/ true,
+                        /*useDwarfDirectory*/ true, IP, std::move(CE),


Suggested change

Ctx, std::move(FOut), /*asmverbose*/ true,

/*useDwarfDirectory*/ true, IP, std::move(CE),

Ctx, std::move(FOut), /*asmverbose=*/ true,

/*useDwarfDirectory=*/ true, IP, std::move(CE),

arsenm · 2024-05-08T07:51:40Z

Utilize -SAME: directives?

If you use -SAME, you still have to produce 2 check lines for the 2 entirely different encoding comment formats. That's not really an improvement if the printed format isn't identical

github-actions · 2024-05-08T14:50:33Z

✅ With the latest revision this PR passed the C/C++ code formatter.

kosarev

llvm-objdump seems to places the encodings as comments. You can check them with -SAME:.

Not until it's disassembled, I'm afraid, which is another RUN line, which is invovenient. And so is having two check lines where one would do, as Matt pointed out. Having two separate check lines also means they may match what is not a complete instruction, e.g., flat_load_d16_b16 v5, v[1:2] where the actual instruction may be flat_load_d16_b16 v5, v[1:2] offset:0.

These details apart, I also must admit I don't quite follow this view that testing is not something of sufficient importance to be properly reflected in our code infrastructure and that it should instead seek its ways around, such as by using llvm-objdump. llvm-mc is, by design, a tool for internal use, testing and debugging, our interface to the MC layer, and I would expect it to cover whatever use cases we developers have for it. Same for specifically the MCAsmStreamer's logic emitting instruction codes, which by the way includes verbalisation of relocations/fix-ups that I'm not sure how we could test using llvm-objdump.

kosarev · 2024-05-08T14:46:55Z

llvm/lib/MC/MCHexStreamer.cpp

+  SmallVector<MCFixup, 4> Fixups;
+  getAssembler().getEmitter().encodeInstruction(Inst, Code, Fixups, STI);
+
+  OS << "[";


kosarev · 2024-05-08T14:47:01Z

llvm/lib/MC/MCHexStreamer.cpp

+
+  OS << "[";
+  for (auto [I, B] : enumerate(Code))
+    OS << (I != 0 ? "," : "") << format("0x%02x", B & 0xff);


kosarev · 2024-05-08T14:47:06Z

llvm/lib/MC/MCHexStreamer.cpp

+  else
+    InstPrinter->printInst(&Inst, 0, "", STI, OS);
+
+  OS << "\n";


kosarev · 2024-05-08T14:48:53Z

llvm/tools/llvm-mc/llvm-mc.cpp

+                        Ctx, std::move(FOut), /*asmverbose*/ true,
+                        /*useDwarfDirectory*/ true, IP, std::move(CE),


Eliminates the need to replicate the same instructions in MC and MC/Disassembler tests and synchronize changes in them. Also highlights differences between disassembled, reassembled and original instructions.

kosarev · 2024-05-17T10:04:55Z

Ping.

MaskRay · 2024-05-17T21:25:09Z

Not until it's disassembled, I'm afraid, which is another RUN line, which is invovenient. And so is having two check lines where one would do, as Matt pointed out. Having two separate check lines also means they may match what is not a complete instruction, e.g., flat_load_d16_b16 v5, v[1:2] where the actual instruction may be flat_load_d16_b16 v5, v[1:2] offset:0.

These details apart, I also must admit I don't quite follow this view that testing is not something of sufficient importance to be properly reflected in our code infrastructure and that it should instead seek its ways around, such as by using llvm-objdump. llvm-mc is, by design, a tool for internal use, testing and debugging, our interface to the MC layer, and I would expect it to cover whatever use cases we developers have for it. Same for specifically the MCAsmStreamer's logic emitting instruction codes, which by the way includes verbalisation of relocations/fix-ups that I'm not sure how we could test using llvm-objdump.

Not sure I am following. It seems that -filetype=hex output is piped to llvm-mc -disassemble.

llvm-mc -filetype=hex a.s | llvm-mc -disassemble

How is this more convenient than llvm-mc -filetype=obj a.s | llvm-objdump -d -?

Can you analyze a mini example including the assembly source and llvm-mc/llvm-objdump command lines for clearer evaluation?

kosarev · 2024-05-30T13:26:36Z

Not sure I am following. It seems that -filetype=hex output is piped to llvm-mc -disassemble.
llvm-mc -filetype=hex a.s | llvm-mc -disassemble
How is this more convenient than llvm-mc -filetype=obj a.s | llvm-objdump -d -?

I think this was already asked and answered above. To reiterate, llvm-mc -disassemble produces its output in the same format as llvm-mc, so we can use the same check directives for both assembler and dissasembler tests. Using llvm-objdump -d - would require multiple directives per instruction, which is not an improvement -- on top of other inconveniences explained earlier.

Can you analyze a mini example including the assembly source and llvm-mc/llvm-objdump command lines for clearer evaluation?

I'm not sure I understand the question. llvm/test/MC/AMDGPU/gfx12_asm_vop1.s in this pull request should hopefully serve as an example, if that's the ask.

MaskRay · 2024-07-05T21:17:35Z

#92895, which does not add another MCStreamer, looks fine to me.
Note, we really should not add another MCStreamer, which is very easy to be misused.

MCAsmStreamer has some existing issues related to expression evaluation. If another MCStreamer is added, I am concerned others might use it for more assembler tasks (e.g. to support .section), which make future maintenance more difficult.
If the llvm-objdump output isn't convenient for AMDGPU, I think it is fine to add a new output mode.

arsenm · 2024-07-24T16:28:47Z

Can this be closed now?

kosarev · 2024-07-25T12:43:55Z

Abandoning this in favour of #92895.

kosarev requested review from jayfoad, rampitec and mbrkusanin April 26, 2024 13:53

llvmbot added backend:AMDGPU mc Machine (object) code labels Apr 26, 2024

kosarev mentioned this pull request Apr 26, 2024

[AMDGPU][RFC] Combine asm and disasm tests. #90179

Closed

MaskRay requested changes Apr 26, 2024

View reviewed changes

arsenm reviewed May 8, 2024

View reviewed changes

kosarev force-pushed the combine-tests2 branch from d12919a to 9045f01 Compare May 8, 2024 14:46

kosarev commented May 8, 2024

View reviewed changes

[AMDGPU][RFC] Combine asm and disasm tests.

9781132

Eliminates the need to replicate the same instructions in MC and MC/Disassembler tests and synchronize changes in them. Also highlights differences between disassembled, reassembled and original instructions.

kosarev force-pushed the combine-tests2 branch from 9045f01 to 9781132 Compare May 8, 2024 14:55

kosarev mentioned this pull request May 21, 2024

[AMDGPU][RFC] Combine asm and disasm tests. #92895

Merged

kosarev closed this Jul 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AMDGPU][RFC] Combine asm and disasm tests. #90214

[AMDGPU][RFC] Combine asm and disasm tests. #90214

kosarev commented Apr 26, 2024

llvmbot commented Apr 26, 2024 •

edited

Loading

MaskRay commented Apr 26, 2024

MaskRay left a comment

kosarev commented Apr 30, 2024

MaskRay commented May 1, 2024

kosarev commented May 1, 2024

MaskRay commented May 1, 2024

kosarev commented May 1, 2024

kosarev commented May 1, 2024

MaskRay commented May 3, 2024

arsenm May 8, 2024

kosarev May 8, 2024

arsenm May 8, 2024

kosarev May 8, 2024

arsenm May 8, 2024

kosarev May 8, 2024

arsenm May 8, 2024

kosarev May 8, 2024

arsenm commented May 8, 2024

github-actions bot commented May 8, 2024 •

edited

Loading

kosarev left a comment

kosarev May 8, 2024

kosarev May 8, 2024

kosarev May 8, 2024

kosarev May 8, 2024

kosarev commented May 17, 2024

MaskRay commented May 17, 2024

kosarev commented May 30, 2024

MaskRay commented Jul 5, 2024

arsenm commented Jul 24, 2024

kosarev commented Jul 25, 2024

		Ctx, std::move(FOut), /asmverbose/ true,
		/useDwarfDirectory/ true, IP, std::move(CE),

[AMDGPU][RFC] Combine asm and disasm tests. #90214

[AMDGPU][RFC] Combine asm and disasm tests. #90214

Conversation

kosarev commented Apr 26, 2024

llvmbot commented Apr 26, 2024 • edited Loading

MaskRay commented Apr 26, 2024

MaskRay left a comment

Choose a reason for hiding this comment

kosarev commented Apr 30, 2024

MaskRay commented May 1, 2024

kosarev commented May 1, 2024

MaskRay commented May 1, 2024

kosarev commented May 1, 2024

kosarev commented May 1, 2024

MaskRay commented May 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arsenm commented May 8, 2024

github-actions bot commented May 8, 2024 • edited Loading

kosarev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kosarev commented May 17, 2024

MaskRay commented May 17, 2024

kosarev commented May 30, 2024

MaskRay commented Jul 5, 2024

arsenm commented Jul 24, 2024

kosarev commented Jul 25, 2024

llvmbot commented Apr 26, 2024 •

edited

Loading

github-actions bot commented May 8, 2024 •

edited

Loading