[llvm] [AMDGPU][MC] Allow UC_VERSION_* constant reuse (PR #96461)
Carl Ritson via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 1 23:41:01 PDT 2024
https://github.com/perlfu updated https://github.com/llvm/llvm-project/pull/96461
>From 88595f24da33f71db1ea33c08b5e5fdb03a3ab0b 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/3] [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 76a559c9443bd..d61e8ebb866df 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 053a7341bb0beb9f7c21ff29454cf70334068120 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/3] 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 d61e8ebb866df..875d1a16a1d56 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 c5cea117d5ca3a25a9840ca63bdb9adae4aa6ddf 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/3] 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 875d1a16a1d56..03541f7e1c5ee 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 694cd7a9bfd28..0724b6f7dd485 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 0000000000000..1fc7fd79548ad
--- /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 be8ff572e6f7d..b62d710b65629 100644
--- a/llvm/unittests/MC/AMDGPU/CMakeLists.txt
+++ b/llvm/unittests/MC/AMDGPU/CMakeLists.txt
@@ -6,15 +6,18 @@ include_directories(
set(LLVM_LINK_COMPONENTS
AMDGPUCodeGen
AMDGPUDesc
+ AMDGPUDisassembler
AMDGPUInfo
CodeGen
Core
MC
+ MCDisassembler
Support
TargetParser
)
add_llvm_unittest(AMDGPUMCTests
+ Disassembler.cpp
DwarfRegMappings.cpp
SIProgramInfoMCExprs.cpp
)
diff --git a/llvm/unittests/MC/AMDGPU/Disassembler.cpp b/llvm/unittests/MC/AMDGPU/Disassembler.cpp
new file mode 100644
index 0000000000000..2ae5e497cdadb
--- /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");
+ }
+}
More information about the llvm-commits
mailing list