[llvm] [AMDGPU][MC] Allow UC_VERSION_* constant reuse (PR #96461)

Carl Ritson via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 2 17:29:12 PDT 2024


https://github.com/perlfu updated https://github.com/llvm/llvm-project/pull/96461

>From 5ca8bbd21ad0428ae9450528d2b113e0980d68a4 Mon Sep 17 00:00:00 2001
From: Carl Ritson <carl.ritson at amd.com>
Date: Mon, 24 Jun 2024 17:32:41 +0900
Subject: [PATCH 1/4] [AMDGPU][MC] Allow UC_VERSION_* constant reuse

If more than one disassembler is created for a context then
allow reuse of existing constants.
Update assertion to validate constant contents.
---
 .../lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
index 0c0e9163fc6ef6..d949999464104e 100644
--- a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
+++ b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
@@ -2366,8 +2366,12 @@ const MCExpr *AMDGPUDisassembler::createConstantSymbolExpr(StringRef Id,
                                                            int64_t Val) {
   MCContext &Ctx = getContext();
   MCSymbol *Sym = Ctx.getOrCreateSymbol(Id);
-  assert(!Sym->isVariable());
-  Sym->setVariableValue(MCConstantExpr::create(Val, Ctx));
+  int64_t Res = ~Val;
+  assert(!Sym->isVariable() ||
+         (Sym->getVariableValue()->evaluateAsAbsolute(Res) && Res == Val));
+  (void)Res;
+  if (!Sym->isVariable())
+    Sym->setVariableValue(MCConstantExpr::create(Val, Ctx));
   return MCSymbolRefExpr::create(Sym, Ctx);
 }
 

>From bb5c51054e4c8b8811859cab7e968226ec6348e4 Mon Sep 17 00:00:00 2001
From: Carl Ritson <carl.ritson at amd.com>
Date: Tue, 25 Jun 2024 09:39:50 +0900
Subject: [PATCH 2/4] Address reviewer feedback: - Change assertion to comment

---
 llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
index d949999464104e..189b4ec37875e1 100644
--- a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
+++ b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
@@ -2366,10 +2366,9 @@ const MCExpr *AMDGPUDisassembler::createConstantSymbolExpr(StringRef Id,
                                                            int64_t Val) {
   MCContext &Ctx = getContext();
   MCSymbol *Sym = Ctx.getOrCreateSymbol(Id);
-  int64_t Res = ~Val;
-  assert(!Sym->isVariable() ||
-         (Sym->getVariableValue()->evaluateAsAbsolute(Res) && Res == Val));
-  (void)Res;
+  // Note: only set value to Val on a new symbol.
+  // Existing symbol may potentially have a different value to the one
+  // requested, which allows for user redefinition of symbols.
   if (!Sym->isVariable())
     Sym->setVariableValue(MCConstantExpr::create(Val, Ctx));
   return MCSymbolRefExpr::create(Sym, Ctx);

>From 1280232517abc0ac0a4f739588cbeb1f47dc8ad6 Mon Sep 17 00:00:00 2001
From: Carl Ritson <carl.ritson at amd.com>
Date: Tue, 2 Jul 2024 15:33:12 +0900
Subject: [PATCH 3/4] Add tests for UC_VERSION override at assembly and in
 custom disassembly.

---
 .../Disassembler/AMDGPUDisassembler.cpp       |  35 +--
 .../AMDGPU/Disassembler/AMDGPUDisassembler.h  |   4 +-
 llvm/test/MC/AMDGPU/custom-uc-version.s       |  41 ++++
 llvm/unittests/MC/AMDGPU/CMakeLists.txt       |   3 +
 llvm/unittests/MC/AMDGPU/Disassembler.cpp     | 202 ++++++++++++++++++
 5 files changed, 271 insertions(+), 14 deletions(-)
 create mode 100644 llvm/test/MC/AMDGPU/custom-uc-version.s
 create mode 100644 llvm/unittests/MC/AMDGPU/Disassembler.cpp

diff --git a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
index 189b4ec37875e1..48abe2d4623388 100644
--- a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
+++ b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
@@ -55,7 +55,7 @@ AMDGPUDisassembler::AMDGPUDisassembler(const MCSubtargetInfo &STI,
     report_fatal_error("Disassembly not yet supported for subtarget");
 
   for (auto [Symbol, Code] : AMDGPU::UCVersion::getGFXVersions())
-    createConstantSymbolExpr(Symbol, Code);
+    UCVersionSymbols.push_back(createConstantSymbolExpr(Symbol, Code));
 
   UCVersionW64Expr = createConstantSymbolExpr("UC_VERSION_W64_BIT", 0x2000);
   UCVersionW32Expr = createConstantSymbolExpr("UC_VERSION_W32_BIT", 0x4000);
@@ -1755,18 +1755,25 @@ MCOperand AMDGPUDisassembler::decodeVersionImm(unsigned Imm) const {
   if (Encoding::encode(Version, W64, W32, MDP) != Imm)
     return MCOperand::createImm(Imm);
 
-  const auto &Versions = AMDGPU::UCVersion::getGFXVersions();
-  auto I = find_if(Versions,
-                   [Version = Version](const AMDGPU::UCVersion::GFXVersion &V) {
-                     return V.Code == Version;
-                   });
+  // Locate UC_VERSION symbol matching Version.
   MCContext &Ctx = getContext();
-  const MCExpr *E;
-  if (I == Versions.end())
+  const MCExpr *E = nullptr;
+  for (auto *VersionSym : UCVersionSymbols) {
+    int64_t Val;
+    if (!VersionSym->evaluateAsAbsolute(Val))
+      continue;
+    if (Val != (int64_t)Version)
+      continue;
+    auto *Sym = Ctx.getOrCreateSymbol(VersionSym->getSymbol().getName());
+    E = MCSymbolRefExpr::create(Sym, Ctx);
+    break;
+  }
+
+  // Default to constant value if not found.
+  if (!E)
     E = MCConstantExpr::create(Version, Ctx);
-  else
-    E = MCSymbolRefExpr::create(Ctx.getOrCreateSymbol(I->Symbol), Ctx);
 
+  // Apply bits.
   if (W64)
     E = MCBinaryExpr::createOr(E, UCVersionW64Expr, Ctx);
   if (W32)
@@ -2362,15 +2369,17 @@ Expected<bool> AMDGPUDisassembler::onSymbolStart(SymbolInfoTy &Symbol,
   return false;
 }
 
-const MCExpr *AMDGPUDisassembler::createConstantSymbolExpr(StringRef Id,
-                                                           int64_t Val) {
+const MCSymbolRefExpr *
+AMDGPUDisassembler::createConstantSymbolExpr(StringRef Id, int64_t Val) {
   MCContext &Ctx = getContext();
   MCSymbol *Sym = Ctx.getOrCreateSymbol(Id);
   // Note: only set value to Val on a new symbol.
   // Existing symbol may potentially have a different value to the one
   // requested, which allows for user redefinition of symbols.
-  if (!Sym->isVariable())
+  if (!Sym->isVariable()) {
+    Sym->setRedefinable(true);
     Sym->setVariableValue(MCConstantExpr::create(Val, Ctx));
+  }
   return MCSymbolRefExpr::create(Sym, Ctx);
 }
 
diff --git a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h
index 694cd7a9bfd282..0724b6f7dd4852 100644
--- a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h
+++ b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h
@@ -30,6 +30,7 @@ class MCAsmInfo;
 class MCInst;
 class MCOperand;
 class MCSubtargetInfo;
+class MCSymbolRefExpr;
 class Twine;
 
 // Exposes an interface expected by autogenerated code in
@@ -105,8 +106,9 @@ class AMDGPUDisassembler : public MCDisassembler {
   const MCExpr *UCVersionW64Expr;
   const MCExpr *UCVersionW32Expr;
   const MCExpr *UCVersionMDPExpr;
+  SmallVector<const MCSymbolRefExpr *> UCVersionSymbols;
 
-  const MCExpr *createConstantSymbolExpr(StringRef Id, int64_t Val);
+  const MCSymbolRefExpr *createConstantSymbolExpr(StringRef Id, int64_t Val);
 
 public:
   AMDGPUDisassembler(const MCSubtargetInfo &STI, MCContext &Ctx,
diff --git a/llvm/test/MC/AMDGPU/custom-uc-version.s b/llvm/test/MC/AMDGPU/custom-uc-version.s
new file mode 100644
index 00000000000000..1fc7fd79548ade
--- /dev/null
+++ b/llvm/test/MC/AMDGPU/custom-uc-version.s
@@ -0,0 +1,41 @@
+// RUN: llvm-mc -triple=amdgcn -show-encoding -mcpu=gfx1010 %s | FileCheck --check-prefix=GFX10 %s
+
+// Test that UC_VERSION* symbols can be redefined.
+
+.set UC_VERSION_GFX10, 99
+
+s_version UC_VERSION_GFX10
+// GFX10: encoding: [0x63,0x00,0x80,0xb0]
+
+s_version UC_VERSION_GFX10 | UC_VERSION_W32_BIT
+// GFX10: encoding: [0x63,0x40,0x80,0xb0]
+
+s_version UC_VERSION_GFX10 | UC_VERSION_W64_BIT
+// GFX10: encoding: [0x63,0x20,0x80,0xb0]
+
+s_version UC_VERSION_GFX10 | UC_VERSION_MDP_BIT
+// GFX10: encoding: [0x63,0x80,0x80,0xb0]
+
+s_version UC_VERSION_GFX10 | UC_VERSION_W64_BIT | UC_VERSION_MDP_BIT
+// GFX10: encoding: [0x63,0xa0,0x80,0xb0]
+
+.set UC_VERSION_GFX10, 100
+.set UC_VERSION_W32_BIT, 0
+.set UC_VERSION_W64_BIT, 0
+.set UC_VERSION_MDP_BIT, 0
+
+s_version UC_VERSION_GFX10
+// GFX10: encoding: [0x64,0x00,0x80,0xb0]
+
+s_version UC_VERSION_GFX10 | UC_VERSION_W32_BIT
+// GFX10: encoding: [0x64,0x00,0x80,0xb0]
+
+s_version UC_VERSION_GFX10 | UC_VERSION_W64_BIT
+// GFX10: encoding: [0x64,0x00,0x80,0xb0]
+
+s_version UC_VERSION_GFX10 | UC_VERSION_MDP_BIT
+// GFX10: encoding: [0x64,0x00,0x80,0xb0]
+
+s_version UC_VERSION_GFX10 | UC_VERSION_W64_BIT | UC_VERSION_MDP_BIT
+// GFX10: encoding: [0x64,0x00,0x80,0xb0]
+
diff --git a/llvm/unittests/MC/AMDGPU/CMakeLists.txt b/llvm/unittests/MC/AMDGPU/CMakeLists.txt
index 0a399772e019e7..029f814a7d510e 100644
--- a/llvm/unittests/MC/AMDGPU/CMakeLists.txt
+++ b/llvm/unittests/MC/AMDGPU/CMakeLists.txt
@@ -6,12 +6,15 @@ include_directories(
 set(LLVM_LINK_COMPONENTS
   AMDGPUCodeGen
   AMDGPUDesc
+  AMDGPUDisassembler
   AMDGPUInfo
   MC
+  MCDisassembler
   Support
   TargetParser
   )
 
 add_llvm_unittest(AMDGPUMCTests
+  Disassembler.cpp
   DwarfRegMappings.cpp
   )
diff --git a/llvm/unittests/MC/AMDGPU/Disassembler.cpp b/llvm/unittests/MC/AMDGPU/Disassembler.cpp
new file mode 100644
index 00000000000000..2ae5e497cdadbf
--- /dev/null
+++ b/llvm/unittests/MC/AMDGPU/Disassembler.cpp
@@ -0,0 +1,202 @@
+//===- llvm/unittest/unittests/MC/AMDGPU/Disassembler.cpp -----------------===//
+//
+// 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-c/Disassembler.h"
+#include "llvm/MC/MCAsmInfo.h"
+#include "llvm/MC/MCContext.h"
+#include "llvm/MC/MCDisassembler/MCDisassembler.h"
+#include "llvm/MC/MCDisassembler/MCSymbolizer.h"
+#include "llvm/MC/MCInst.h"
+#include "llvm/MC/MCInstPrinter.h"
+#include "llvm/MC/MCInstrInfo.h"
+#include "llvm/MC/MCRegisterInfo.h"
+#include "llvm/MC/MCSubtargetInfo.h"
+#include "llvm/MC/MCSymbol.h"
+#include "llvm/MC/MCTargetOptions.h"
+#include "llvm/MC/TargetRegistry.h"
+#include "llvm/Support/TargetSelect.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+static const char *symbolLookupCallback(void *DisInfo, uint64_t ReferenceValue,
+                                        uint64_t *ReferenceType,
+                                        uint64_t ReferencePC,
+                                        const char **ReferenceName) {
+  *ReferenceType = LLVMDisassembler_ReferenceType_InOut_None;
+  return nullptr;
+}
+
+static const char *TripleName = "amdgcn--amdpal";
+static const char *CPUName = "gfx1030";
+
+// Basic smoke test.
+TEST(AMDGPUDisassembler, Basic) {
+  LLVMInitializeAMDGPUTargetInfo();
+  LLVMInitializeAMDGPUTargetMC();
+  LLVMInitializeAMDGPUDisassembler();
+
+  uint8_t Bytes[] = {0x04, 0x00, 0x80, 0xb0};
+  uint8_t *BytesP = Bytes;
+  const char OutStringSize = 100;
+  char OutString[OutStringSize];
+  LLVMDisasmContextRef DCR = LLVMCreateDisasmCPU(
+      TripleName, CPUName, nullptr, 0, nullptr, symbolLookupCallback);
+
+  // Skip test if AMDGPU not built.
+  if (!DCR)
+    GTEST_SKIP();
+
+  size_t InstSize;
+  unsigned NumBytes = sizeof(Bytes);
+  unsigned PC = 0U;
+
+  InstSize = LLVMDisasmInstruction(DCR, BytesP, NumBytes, PC, OutString,
+                                   OutStringSize);
+  EXPECT_EQ(InstSize, 4U);
+  EXPECT_EQ(StringRef(OutString), "\ts_version UC_VERSION_GFX10");
+
+  LLVMDisasmDispose(DCR);
+}
+
+// Check multiple disassemblers in same MCContext.
+TEST(AMDGPUDisassembler, MultiDisassembler) {
+  LLVMInitializeAMDGPUTargetInfo();
+  LLVMInitializeAMDGPUTargetMC();
+  LLVMInitializeAMDGPUDisassembler();
+
+  std::string Error;
+  const Target *TheTarget = TargetRegistry::lookupTarget(TripleName, Error);
+
+  // Skip test if AMDGPU not built.
+  if (!TheTarget)
+    GTEST_SKIP();
+
+  std::unique_ptr<MCRegisterInfo> MRI(TheTarget->createMCRegInfo(TripleName));
+  std::unique_ptr<MCAsmInfo> MAI(
+      TheTarget->createMCAsmInfo(*MRI, TripleName, MCTargetOptions()));
+  std::unique_ptr<const MCInstrInfo> MII(TheTarget->createMCInstrInfo());
+  std::unique_ptr<MCSubtargetInfo> STI(
+      TheTarget->createMCSubtargetInfo(TripleName, CPUName, ""));
+  auto Ctx = std::make_unique<MCContext>(Triple(TripleName), MAI.get(),
+                                         MRI.get(), STI.get());
+
+  int AsmPrinterVariant = MAI->getAssemblerDialect();
+  std::unique_ptr<MCInstPrinter> IP(TheTarget->createMCInstPrinter(
+      Triple(TripleName), AsmPrinterVariant, *MAI, *MII, *MRI));
+
+  SmallVector<char, 64> InsnStr, AnnoStr;
+  raw_svector_ostream OS(InsnStr);
+  raw_svector_ostream Annotations(AnnoStr);
+  formatted_raw_ostream FormattedOS(OS);
+
+  char StrBuffer[128];
+
+  uint8_t Bytes[] = {0x04, 0x00, 0x80, 0xb0};
+  size_t InstSize = 0U;
+  MCInst Inst1, Inst2;
+  MCDisassembler::DecodeStatus Status;
+
+  // Test disassembler works as expected.
+  AnnoStr.clear();
+  InsnStr.clear();
+  std::unique_ptr<MCDisassembler> DisAsm1(
+      TheTarget->createMCDisassembler(*STI, *Ctx));
+  Status = DisAsm1->getInstruction(Inst1, InstSize, Bytes, 0, Annotations);
+  ASSERT_TRUE(Status == MCDisassembler::Success);
+  EXPECT_EQ(InstSize, 4U);
+
+  IP->printInst(&Inst1, 0U, Annotations.str(), *STI, FormattedOS);
+  ASSERT_TRUE(InsnStr.size() < (sizeof(StrBuffer) - 1));
+  std::memcpy(StrBuffer, InsnStr.data(), InsnStr.size());
+  StrBuffer[InsnStr.size()] = '\0';
+  EXPECT_EQ(StringRef(StrBuffer), "\ts_version UC_VERSION_GFX10");
+
+  // Test that second disassembler in same context works as expected.
+  AnnoStr.clear();
+  InsnStr.clear();
+  std::unique_ptr<MCDisassembler> DisAsm2(
+      TheTarget->createMCDisassembler(*STI, *Ctx));
+  Status = DisAsm2->getInstruction(Inst2, InstSize, Bytes, 0, Annotations);
+  ASSERT_TRUE(Status == MCDisassembler::Success);
+  EXPECT_EQ(InstSize, 4U);
+
+  IP->printInst(&Inst2, 0U, Annotations.str(), *STI, FormattedOS);
+  ASSERT_TRUE(InsnStr.size() < (sizeof(StrBuffer) - 1));
+  std::memcpy(StrBuffer, InsnStr.data(), InsnStr.size());
+  StrBuffer[InsnStr.size()] = '\0';
+  EXPECT_EQ(StringRef(StrBuffer), "\ts_version UC_VERSION_GFX10");
+}
+
+// Test UC_VERSION symbols can be overriden.
+TEST(AMDGPUDisassembler, UCVersionOverride) {
+  LLVMInitializeAMDGPUTargetInfo();
+  LLVMInitializeAMDGPUTargetMC();
+  LLVMInitializeAMDGPUDisassembler();
+
+  std::string Error;
+  const Target *TheTarget = TargetRegistry::lookupTarget(TripleName, Error);
+
+  // Skip test if AMDGPU not built.
+  if (!TheTarget)
+    GTEST_SKIP();
+
+  std::unique_ptr<MCRegisterInfo> MRI(TheTarget->createMCRegInfo(TripleName));
+  std::unique_ptr<MCAsmInfo> MAI(
+      TheTarget->createMCAsmInfo(*MRI, TripleName, MCTargetOptions()));
+  std::unique_ptr<const MCInstrInfo> MII(TheTarget->createMCInstrInfo());
+  std::unique_ptr<MCSubtargetInfo> STI(
+      TheTarget->createMCSubtargetInfo(TripleName, CPUName, ""));
+  auto Ctx = std::make_unique<MCContext>(Triple(TripleName), MAI.get(),
+                                         MRI.get(), STI.get());
+
+  // Define custom UC_VERSION before initializing disassembler.
+  const uint8_t UC_VERSION_GFX10_DEFAULT = 0x04;
+  const uint8_t UC_VERSION_GFX10_NEW = 0x99;
+  auto Sym = Ctx->getOrCreateSymbol("UC_VERSION_GFX10");
+  Sym->setVariableValue(MCConstantExpr::create(UC_VERSION_GFX10_NEW, *Ctx));
+
+  int AsmPrinterVariant = MAI->getAssemblerDialect();
+  std::unique_ptr<MCInstPrinter> IP(TheTarget->createMCInstPrinter(
+      Triple(TripleName), AsmPrinterVariant, *MAI, *MII, *MRI));
+
+  std::unique_ptr<MCDisassembler> DisAsm(
+      TheTarget->createMCDisassembler(*STI, *Ctx));
+
+  SmallVector<char, 64> InsnStr, AnnoStr;
+  raw_svector_ostream OS(InsnStr);
+  raw_svector_ostream Annotations(AnnoStr);
+  formatted_raw_ostream FormattedOS(OS);
+
+  char StrBuffer[128];
+
+  // Decode S_VERSION instruction with original or custom version.
+  uint8_t Versions[] = {UC_VERSION_GFX10_DEFAULT, UC_VERSION_GFX10_NEW};
+  for (uint8_t Version : Versions) {
+    uint8_t Bytes[] = {Version, 0x00, 0x80, 0xb0};
+    size_t InstSize = 0U;
+    MCInst Inst;
+
+    AnnoStr.clear();
+    InsnStr.clear();
+    MCDisassembler::DecodeStatus Status =
+        DisAsm->getInstruction(Inst, InstSize, Bytes, 0, Annotations);
+    ASSERT_TRUE(Status == MCDisassembler::Success);
+    EXPECT_EQ(InstSize, 4U);
+
+    IP->printInst(&Inst, 0, Annotations.str(), *STI, FormattedOS);
+    ASSERT_TRUE(InsnStr.size() < (sizeof(StrBuffer) - 1));
+    std::memcpy(StrBuffer, InsnStr.data(), InsnStr.size());
+    StrBuffer[InsnStr.size()] = '\0';
+
+    if (Version == UC_VERSION_GFX10_DEFAULT)
+      EXPECT_EQ(StringRef(StrBuffer), "\ts_version 4");
+    else
+      EXPECT_EQ(StringRef(StrBuffer), "\ts_version UC_VERSION_GFX10");
+  }
+}

>From cbbc9b5a9f20d3875f9a05f6208bb34f1eb5998d Mon Sep 17 00:00:00 2001
From: Carl Ritson <carl.ritson at amd.com>
Date: Wed, 3 Jul 2024 09:22:24 +0900
Subject: [PATCH 4/4] Address reviewer feedback.

---
 .../Disassembler/AMDGPUDisassembler.cpp       | 43 +++++-------
 .../AMDGPU/Disassembler/AMDGPUDisassembler.h  |  4 +-
 llvm/test/MC/AMDGPU/custom-uc-version.s       | 41 -----------
 llvm/unittests/MC/AMDGPU/Disassembler.cpp     | 68 -------------------
 4 files changed, 19 insertions(+), 137 deletions(-)
 delete mode 100644 llvm/test/MC/AMDGPU/custom-uc-version.s

diff --git a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
index 48abe2d4623388..1fb5e73db0da0f 100644
--- a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
+++ b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
@@ -55,7 +55,7 @@ AMDGPUDisassembler::AMDGPUDisassembler(const MCSubtargetInfo &STI,
     report_fatal_error("Disassembly not yet supported for subtarget");
 
   for (auto [Symbol, Code] : AMDGPU::UCVersion::getGFXVersions())
-    UCVersionSymbols.push_back(createConstantSymbolExpr(Symbol, Code));
+    createConstantSymbolExpr(Symbol, Code);
 
   UCVersionW64Expr = createConstantSymbolExpr("UC_VERSION_W64_BIT", 0x2000);
   UCVersionW32Expr = createConstantSymbolExpr("UC_VERSION_W32_BIT", 0x4000);
@@ -1755,25 +1755,18 @@ MCOperand AMDGPUDisassembler::decodeVersionImm(unsigned Imm) const {
   if (Encoding::encode(Version, W64, W32, MDP) != Imm)
     return MCOperand::createImm(Imm);
 
-  // Locate UC_VERSION symbol matching Version.
+  const auto &Versions = AMDGPU::UCVersion::getGFXVersions();
+  auto I = find_if(Versions,
+                   [Version = Version](const AMDGPU::UCVersion::GFXVersion &V) {
+                     return V.Code == Version;
+                   });
   MCContext &Ctx = getContext();
-  const MCExpr *E = nullptr;
-  for (auto *VersionSym : UCVersionSymbols) {
-    int64_t Val;
-    if (!VersionSym->evaluateAsAbsolute(Val))
-      continue;
-    if (Val != (int64_t)Version)
-      continue;
-    auto *Sym = Ctx.getOrCreateSymbol(VersionSym->getSymbol().getName());
-    E = MCSymbolRefExpr::create(Sym, Ctx);
-    break;
-  }
-
-  // Default to constant value if not found.
-  if (!E)
+  const MCExpr *E;
+  if (I == Versions.end())
     E = MCConstantExpr::create(Version, Ctx);
+  else
+    E = MCSymbolRefExpr::create(Ctx.getOrCreateSymbol(I->Symbol), Ctx);
 
-  // Apply bits.
   if (W64)
     E = MCBinaryExpr::createOr(E, UCVersionW64Expr, Ctx);
   if (W32)
@@ -2369,17 +2362,17 @@ Expected<bool> AMDGPUDisassembler::onSymbolStart(SymbolInfoTy &Symbol,
   return false;
 }
 
-const MCSymbolRefExpr *
-AMDGPUDisassembler::createConstantSymbolExpr(StringRef Id, int64_t Val) {
+const MCExpr *AMDGPUDisassembler::createConstantSymbolExpr(StringRef Id,
+                                                           int64_t Val) {
   MCContext &Ctx = getContext();
   MCSymbol *Sym = Ctx.getOrCreateSymbol(Id);
-  // Note: only set value to Val on a new symbol.
-  // Existing symbol may potentially have a different value to the one
-  // requested, which allows for user redefinition of symbols.
-  if (!Sym->isVariable()) {
-    Sym->setRedefinable(true);
+  // Note: only set value to Val on a new symbol in case an dissassembler
+  // has already been initialized in this context.
+  [[maybe_unused]] int64_t Res = ~Val;
+  assert(!Sym->isVariable() ||
+         (Sym->getVariableValue()->evaluateAsAbsolute(Res) && Res == Val));
+  if (!Sym->isVariable())
     Sym->setVariableValue(MCConstantExpr::create(Val, Ctx));
-  }
   return MCSymbolRefExpr::create(Sym, Ctx);
 }
 
diff --git a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h
index 0724b6f7dd4852..694cd7a9bfd282 100644
--- a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h
+++ b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h
@@ -30,7 +30,6 @@ class MCAsmInfo;
 class MCInst;
 class MCOperand;
 class MCSubtargetInfo;
-class MCSymbolRefExpr;
 class Twine;
 
 // Exposes an interface expected by autogenerated code in
@@ -106,9 +105,8 @@ class AMDGPUDisassembler : public MCDisassembler {
   const MCExpr *UCVersionW64Expr;
   const MCExpr *UCVersionW32Expr;
   const MCExpr *UCVersionMDPExpr;
-  SmallVector<const MCSymbolRefExpr *> UCVersionSymbols;
 
-  const MCSymbolRefExpr *createConstantSymbolExpr(StringRef Id, int64_t Val);
+  const MCExpr *createConstantSymbolExpr(StringRef Id, int64_t Val);
 
 public:
   AMDGPUDisassembler(const MCSubtargetInfo &STI, MCContext &Ctx,
diff --git a/llvm/test/MC/AMDGPU/custom-uc-version.s b/llvm/test/MC/AMDGPU/custom-uc-version.s
deleted file mode 100644
index 1fc7fd79548ade..00000000000000
--- a/llvm/test/MC/AMDGPU/custom-uc-version.s
+++ /dev/null
@@ -1,41 +0,0 @@
-// RUN: llvm-mc -triple=amdgcn -show-encoding -mcpu=gfx1010 %s | FileCheck --check-prefix=GFX10 %s
-
-// Test that UC_VERSION* symbols can be redefined.
-
-.set UC_VERSION_GFX10, 99
-
-s_version UC_VERSION_GFX10
-// GFX10: encoding: [0x63,0x00,0x80,0xb0]
-
-s_version UC_VERSION_GFX10 | UC_VERSION_W32_BIT
-// GFX10: encoding: [0x63,0x40,0x80,0xb0]
-
-s_version UC_VERSION_GFX10 | UC_VERSION_W64_BIT
-// GFX10: encoding: [0x63,0x20,0x80,0xb0]
-
-s_version UC_VERSION_GFX10 | UC_VERSION_MDP_BIT
-// GFX10: encoding: [0x63,0x80,0x80,0xb0]
-
-s_version UC_VERSION_GFX10 | UC_VERSION_W64_BIT | UC_VERSION_MDP_BIT
-// GFX10: encoding: [0x63,0xa0,0x80,0xb0]
-
-.set UC_VERSION_GFX10, 100
-.set UC_VERSION_W32_BIT, 0
-.set UC_VERSION_W64_BIT, 0
-.set UC_VERSION_MDP_BIT, 0
-
-s_version UC_VERSION_GFX10
-// GFX10: encoding: [0x64,0x00,0x80,0xb0]
-
-s_version UC_VERSION_GFX10 | UC_VERSION_W32_BIT
-// GFX10: encoding: [0x64,0x00,0x80,0xb0]
-
-s_version UC_VERSION_GFX10 | UC_VERSION_W64_BIT
-// GFX10: encoding: [0x64,0x00,0x80,0xb0]
-
-s_version UC_VERSION_GFX10 | UC_VERSION_MDP_BIT
-// GFX10: encoding: [0x64,0x00,0x80,0xb0]
-
-s_version UC_VERSION_GFX10 | UC_VERSION_W64_BIT | UC_VERSION_MDP_BIT
-// GFX10: encoding: [0x64,0x00,0x80,0xb0]
-
diff --git a/llvm/unittests/MC/AMDGPU/Disassembler.cpp b/llvm/unittests/MC/AMDGPU/Disassembler.cpp
index 2ae5e497cdadbf..1448685806d783 100644
--- a/llvm/unittests/MC/AMDGPU/Disassembler.cpp
+++ b/llvm/unittests/MC/AMDGPU/Disassembler.cpp
@@ -132,71 +132,3 @@ TEST(AMDGPUDisassembler, MultiDisassembler) {
   StrBuffer[InsnStr.size()] = '\0';
   EXPECT_EQ(StringRef(StrBuffer), "\ts_version UC_VERSION_GFX10");
 }
-
-// Test UC_VERSION symbols can be overriden.
-TEST(AMDGPUDisassembler, UCVersionOverride) {
-  LLVMInitializeAMDGPUTargetInfo();
-  LLVMInitializeAMDGPUTargetMC();
-  LLVMInitializeAMDGPUDisassembler();
-
-  std::string Error;
-  const Target *TheTarget = TargetRegistry::lookupTarget(TripleName, Error);
-
-  // Skip test if AMDGPU not built.
-  if (!TheTarget)
-    GTEST_SKIP();
-
-  std::unique_ptr<MCRegisterInfo> MRI(TheTarget->createMCRegInfo(TripleName));
-  std::unique_ptr<MCAsmInfo> MAI(
-      TheTarget->createMCAsmInfo(*MRI, TripleName, MCTargetOptions()));
-  std::unique_ptr<const MCInstrInfo> MII(TheTarget->createMCInstrInfo());
-  std::unique_ptr<MCSubtargetInfo> STI(
-      TheTarget->createMCSubtargetInfo(TripleName, CPUName, ""));
-  auto Ctx = std::make_unique<MCContext>(Triple(TripleName), MAI.get(),
-                                         MRI.get(), STI.get());
-
-  // Define custom UC_VERSION before initializing disassembler.
-  const uint8_t UC_VERSION_GFX10_DEFAULT = 0x04;
-  const uint8_t UC_VERSION_GFX10_NEW = 0x99;
-  auto Sym = Ctx->getOrCreateSymbol("UC_VERSION_GFX10");
-  Sym->setVariableValue(MCConstantExpr::create(UC_VERSION_GFX10_NEW, *Ctx));
-
-  int AsmPrinterVariant = MAI->getAssemblerDialect();
-  std::unique_ptr<MCInstPrinter> IP(TheTarget->createMCInstPrinter(
-      Triple(TripleName), AsmPrinterVariant, *MAI, *MII, *MRI));
-
-  std::unique_ptr<MCDisassembler> DisAsm(
-      TheTarget->createMCDisassembler(*STI, *Ctx));
-
-  SmallVector<char, 64> InsnStr, AnnoStr;
-  raw_svector_ostream OS(InsnStr);
-  raw_svector_ostream Annotations(AnnoStr);
-  formatted_raw_ostream FormattedOS(OS);
-
-  char StrBuffer[128];
-
-  // Decode S_VERSION instruction with original or custom version.
-  uint8_t Versions[] = {UC_VERSION_GFX10_DEFAULT, UC_VERSION_GFX10_NEW};
-  for (uint8_t Version : Versions) {
-    uint8_t Bytes[] = {Version, 0x00, 0x80, 0xb0};
-    size_t InstSize = 0U;
-    MCInst Inst;
-
-    AnnoStr.clear();
-    InsnStr.clear();
-    MCDisassembler::DecodeStatus Status =
-        DisAsm->getInstruction(Inst, InstSize, Bytes, 0, Annotations);
-    ASSERT_TRUE(Status == MCDisassembler::Success);
-    EXPECT_EQ(InstSize, 4U);
-
-    IP->printInst(&Inst, 0, Annotations.str(), *STI, FormattedOS);
-    ASSERT_TRUE(InsnStr.size() < (sizeof(StrBuffer) - 1));
-    std::memcpy(StrBuffer, InsnStr.data(), InsnStr.size());
-    StrBuffer[InsnStr.size()] = '\0';
-
-    if (Version == UC_VERSION_GFX10_DEFAULT)
-      EXPECT_EQ(StringRef(StrBuffer), "\ts_version 4");
-    else
-      EXPECT_EQ(StringRef(StrBuffer), "\ts_version UC_VERSION_GFX10");
-  }
-}



More information about the llvm-commits mailing list