[llvm] [SPIRV] Fix code quality issues. (PR #152005)
Marcos Maronas via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 5 08:08:58 PDT 2025
https://github.com/maarquitos14 updated https://github.com/llvm/llvm-project/pull/152005
>From 6228c9cbf183d5769efb6089b37287e6d6938ea7 Mon Sep 17 00:00:00 2001
From: Marcos Maronas <marcos.maronas at intel.com>
Date: Mon, 4 Aug 2025 19:05:06 +0200
Subject: [PATCH 1/3] [SPIRV] Fix code quality issues.
---
.../SPIRV/Analysis/SPIRVConvergenceRegionAnalysis.h | 8 ++++++++
.../Target/SPIRV/MCTargetDesc/SPIRVInstPrinter.cpp | 3 ++-
llvm/lib/Target/SPIRV/SPIRVAPI.cpp | 2 +-
llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp | 6 ++++--
llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp | 4 +++-
llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp | 8 +++++---
llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp | 1 +
llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp | 11 +++++++----
llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp | 3 ++-
llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp | 6 +++---
llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h | 7 ++++---
llvm/lib/Target/SPIRV/SPIRVPostLegalizer.cpp | 2 +-
llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp | 1 +
llvm/lib/Target/SPIRV/SPIRVPrepareFunctions.cpp | 2 +-
llvm/lib/Target/SPIRV/SPIRVUtils.cpp | 5 +++--
15 files changed, 46 insertions(+), 23 deletions(-)
diff --git a/llvm/lib/Target/SPIRV/Analysis/SPIRVConvergenceRegionAnalysis.h b/llvm/lib/Target/SPIRV/Analysis/SPIRVConvergenceRegionAnalysis.h
index 78a066bef8abc..ca4b8459e9f36 100644
--- a/llvm/lib/Target/SPIRV/Analysis/SPIRVConvergenceRegionAnalysis.h
+++ b/llvm/lib/Target/SPIRV/Analysis/SPIRVConvergenceRegionAnalysis.h
@@ -73,7 +73,12 @@ class ConvergenceRegion {
Entry(std::move(CR.Entry)), Exits(std::move(CR.Exits)),
Blocks(std::move(CR.Blocks)) {}
+ // Destructor.
+ ~ConvergenceRegion() { releaseMemory(); }
+
+ ConvergenceRegion &operator=(ConvergenceRegion &&CR) = delete;
ConvergenceRegion(const ConvergenceRegion &other) = delete;
+ ConvergenceRegion &operator=(const ConvergenceRegion &other) = delete;
// Returns true if the given basic block belongs to this region, or to one of
// its subregion.
@@ -101,6 +106,9 @@ class ConvergenceRegionInfo {
~ConvergenceRegionInfo() { releaseMemory(); }
+ ConvergenceRegionInfo(const ConvergenceRegionInfo &LHS) = default;
+ ConvergenceRegionInfo &operator=(const ConvergenceRegionInfo &LHS) = default;
+
ConvergenceRegionInfo(ConvergenceRegionInfo &&LHS)
: TopLevelRegion(LHS.TopLevelRegion) {
if (TopLevelRegion != LHS.TopLevelRegion) {
diff --git a/llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVInstPrinter.cpp b/llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVInstPrinter.cpp
index a7f6fbceffc3f..a0024fc186a2d 100644
--- a/llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVInstPrinter.cpp
+++ b/llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVInstPrinter.cpp
@@ -96,7 +96,7 @@ void SPIRVInstPrinter::printOpConstantVarOps(const MCInst *MI,
void SPIRVInstPrinter::recordOpExtInstImport(const MCInst *MI) {
MCRegister Reg = MI->getOperand(0).getReg();
auto Name = getSPIRVStringOperand(*MI, 1);
- auto Set = getExtInstSetFromString(Name);
+ auto Set = getExtInstSetFromString(std::move(Name));
ExtInstSetIDs.insert({Reg, Set});
}
@@ -210,6 +210,7 @@ void SPIRVInstPrinter::printInst(const MCInst *MI, uint64_t Address,
case SPIRV::OpConstantF:
// The last fixed operand along with any variadic operands that follow
// are part of the variable value.
+ assert(NumFixedOps > 0 && "Expected at least one fixed operand");
printOpConstantVarOps(MI, NumFixedOps - 1, OS);
break;
case SPIRV::OpCooperativeMatrixMulAddKHR: {
diff --git a/llvm/lib/Target/SPIRV/SPIRVAPI.cpp b/llvm/lib/Target/SPIRV/SPIRVAPI.cpp
index cfe7ef486381d..d6581b274e00f 100644
--- a/llvm/lib/Target/SPIRV/SPIRVAPI.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVAPI.cpp
@@ -156,7 +156,7 @@ SPIRVTranslateModule(Module *M, std::string &SpirvObj, std::string &ErrMsg,
}
}
return SPIRVTranslate(M, SpirvObj, ErrMsg, AllowExtNames, OLevel,
- TargetTriple);
+ std::move(TargetTriple));
}
} // namespace llvm
diff --git a/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp b/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
index 1ebfde2a603b9..2ada672156d6f 100644
--- a/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
@@ -50,7 +50,8 @@ class SPIRVAsmPrinter : public AsmPrinter {
public:
explicit SPIRVAsmPrinter(TargetMachine &TM,
std::unique_ptr<MCStreamer> Streamer)
- : AsmPrinter(TM, std::move(Streamer), ID), ST(nullptr), TII(nullptr) {}
+ : AsmPrinter(TM, std::move(Streamer), ID), ModuleSectionsEmitted(false),
+ ST(nullptr), TII(nullptr), MAI(nullptr) {}
static char ID;
bool ModuleSectionsEmitted;
const SPIRVSubtarget *ST;
@@ -591,7 +592,8 @@ void SPIRVAsmPrinter::outputAnnotations(const Module &M) {
cast<GlobalVariable>(CS->getOperand(1)->stripPointerCasts());
StringRef AnnotationString;
- getConstantStringInfo(GV, AnnotationString);
+ bool Success = getConstantStringInfo(GV, AnnotationString);
+ assert(Success && "Failed to get annotation string");
MCInst Inst;
Inst.setOpcode(SPIRV::OpDecorate);
Inst.addOperand(MCOperand::createReg(Reg));
diff --git a/llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp b/llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp
index 25cdf72a658a8..6e2f477e80012 100644
--- a/llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp
@@ -51,7 +51,7 @@ struct IncomingCall {
IncomingCall(const std::string BuiltinName, const DemangledBuiltin *Builtin,
const Register ReturnRegister, const SPIRVType *ReturnType,
const SmallVectorImpl<Register> &Arguments)
- : BuiltinName(BuiltinName), Builtin(Builtin),
+ : BuiltinName(std::move(BuiltinName)), Builtin(Builtin),
ReturnRegister(ReturnRegister), ReturnType(ReturnType),
Arguments(Arguments) {}
@@ -2619,6 +2619,7 @@ static bool generateConvertInst(const StringRef DemangledCall,
GR->getSPIRVTypeID(Call->ReturnType));
}
+ assert(Builtin && "Conversion builtin not found.");
if (Builtin->IsSaturated)
buildOpDecorate(Call->ReturnRegister, MIRBuilder,
SPIRV::Decoration::SaturatedConversion, {});
@@ -3186,6 +3187,7 @@ static SPIRVType *getLayoutType(const TargetExtType *ExtensionType,
namespace SPIRV {
TargetExtType *parseBuiltinTypeNameToTargetExtType(std::string TypeName,
LLVMContext &Context) {
+ assert(!TypeName.empty() && "Empty builtin type name!");
StringRef NameWithParameters = TypeName;
// Pointers-to-opaque-structs representing OpenCL types are first translated
diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index 9f55330cb7ac6..239b55ae23e07 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -495,7 +495,7 @@ void SPIRVEmitIntrinsics::propagateElemTypeRec(
std::unordered_set<Value *> Visited;
DenseMap<Function *, CallInst *> Ptrcasts;
propagateElemTypeRec(Op, PtrElemTy, CastElemTy, VisitedSubst, Visited,
- Ptrcasts);
+ std::move(Ptrcasts));
}
void SPIRVEmitIntrinsics::propagateElemTypeRec(
@@ -893,9 +893,11 @@ Type *SPIRVEmitIntrinsics::deduceNestedTypeHelper(
bool Change = false;
for (unsigned i = 0; i < U->getNumOperands(); ++i) {
Value *Op = U->getOperand(i);
- Type *OpTy = Op->getType();
- Type *Ty = OpTy;
+ Type *OpTy = nullptr;
+ Type *Ty = nullptr;
if (Op) {
+ OpTy = Op->getType();
+ Ty = OpTy;
if (auto *PtrTy = dyn_cast<PointerType>(OpTy)) {
if (Type *NestedTy =
deduceElementTypeHelper(Op, Visited, UnknownElemTypeI8))
diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp
index 7f0d636577869..275463ef4c527 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitNonSemanticDI.cpp
@@ -116,6 +116,7 @@ bool SPIRVEmitNonSemanticDI::emitGlobalDI(MachineFunction &MF) {
}
}
const NamedMDNode *ModuleFlags = M->getNamedMetadata("llvm.module.flags");
+ assert(ModuleFlags && "Expected llvm.module.flags metadata to be present");
for (const auto *Op : ModuleFlags->operands()) {
const MDOperand &MaybeStrOp = Op->getOperand(1);
if (MaybeStrOp.equalsStr("Dwarf Version"))
diff --git a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
index 83fccdc2bdba3..a1b3a75d6706d 100644
--- a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
@@ -87,7 +87,7 @@ storageClassRequiresExplictLayout(SPIRV::StorageClass::StorageClass SC) {
}
SPIRVGlobalRegistry::SPIRVGlobalRegistry(unsigned PointerSize)
- : PointerSize(PointerSize), Bound(0) {}
+ : PointerSize(PointerSize), Bound(0), CurMF(nullptr) {}
SPIRVType *SPIRVGlobalRegistry::assignIntTypeToVReg(unsigned BitWidth,
Register VReg,
@@ -474,6 +474,7 @@ Register SPIRVGlobalRegistry::getOrCreateBaseRegister(
}
if (Type->getOpcode() == SPIRV::OpTypeFloat) {
SPIRVType *SpvBaseType = getOrCreateSPIRVFloatType(BitWidth, I, TII);
+ assert(isa<ConstantFP>(Val) && "Expected ConstantFP for OpTypeFloat");
return getOrCreateConstFP(dyn_cast<ConstantFP>(Val)->getValue(), I,
SpvBaseType, TII, ZeroAsNull);
}
@@ -1063,7 +1064,8 @@ SPIRVType *SPIRVGlobalRegistry::createSPIRVType(
MIRBuilder);
};
}
- return getOpTypeStruct(SType, MIRBuilder, AccQual, Decorator, EmitIR);
+ return getOpTypeStruct(SType, MIRBuilder, AccQual, std::move(Decorator),
+ EmitIR);
}
if (auto FType = dyn_cast<FunctionType>(Ty)) {
SPIRVType *RetTy = findSPIRVType(FType->getReturnType(), MIRBuilder,
@@ -1400,8 +1402,9 @@ SPIRVType *SPIRVGlobalRegistry::getOrCreateLayoutType(
// We need a new OpTypeStruct instruction because decorations will be
// different from a struct with an explicit layout created from a different
// entry point.
- SPIRVType *SPIRVStructType = getOpTypeStruct(
- ST, MIRBuilder, SPIRV::AccessQualifier::None, Decorator, EmitIr);
+ SPIRVType *SPIRVStructType =
+ getOpTypeStruct(ST, MIRBuilder, SPIRV::AccessQualifier::None,
+ std::move(Decorator), EmitIr);
add(Key, SPIRVStructType);
return SPIRVStructType;
}
diff --git a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
index e9f5ffa23e220..5259db1ff2dd7 100644
--- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
@@ -362,6 +362,7 @@ SPIRVInstructionSelector::SPIRVInstructionSelector(const SPIRVTargetMachine &TM,
const RegisterBankInfo &RBI)
: InstructionSelector(), STI(ST), TII(*ST.getInstrInfo()),
TRI(*ST.getRegisterInfo()), RBI(RBI), GR(*ST.getSPIRVGlobalRegistry()),
+ MRI(nullptr),
#define GET_GLOBALISEL_PREDICATES_INIT
#include "SPIRVGenGlobalISel.inc"
#undef GET_GLOBALISEL_PREDICATES_INIT
@@ -3574,7 +3575,7 @@ bool SPIRVInstructionSelector::selectFirstBitSet64Overflow(
// Join all the resulting registers back into the return type in order
// (ie i32x2, i32x2, i32x1 -> i32x5)
- return selectOpWithSrcs(ResVReg, ResType, I, PartialRegs,
+ return selectOpWithSrcs(ResVReg, ResType, I, std::move(PartialRegs),
SPIRV::OpCompositeConstruct);
}
diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
index 0cd9d7882a52a..7fce7033a2757 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
@@ -93,7 +93,7 @@ getSymbolicOperandRequirements(SPIRV::OperandCategory::OperandCategory Category,
if (Reqs.isCapabilityAvailable(Cap)) {
ReqExts.append(getSymbolicOperandExtensions(
SPIRV::OperandCategory::CapabilityOperand, Cap));
- return {true, {Cap}, ReqExts, ReqMinVer, ReqMaxVer};
+ return {true, {Cap}, std::move(ReqExts), ReqMinVer, ReqMaxVer};
}
} else {
// By SPIR-V specification: "If an instruction, enumerant, or other
@@ -111,7 +111,7 @@ getSymbolicOperandRequirements(SPIRV::OperandCategory::OperandCategory Category,
if (i == Sz - 1 || !AvoidCaps.S.contains(Cap)) {
ReqExts.append(getSymbolicOperandExtensions(
SPIRV::OperandCategory::CapabilityOperand, Cap));
- return {true, {Cap}, ReqExts, ReqMinVer, ReqMaxVer};
+ return {true, {Cap}, std::move(ReqExts), ReqMinVer, ReqMaxVer};
}
}
}
@@ -558,7 +558,7 @@ static void collectOtherInstr(MachineInstr &MI, SPIRV::ModuleAnalysisInfo &MAI,
bool Append = true) {
MAI.setSkipEmission(&MI);
InstrSignature MISign = instrToSignature(MI, MAI, true);
- auto FoundMI = IS.insert(MISign);
+ auto FoundMI = IS.insert(std::move(MISign));
if (!FoundMI.second)
return; // insert failed, so we found a duplicate; don't add it to MAI.MS
// No duplicates, so add it.
diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h
index a0d47cb052b42..41c792a98534f 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h
@@ -54,8 +54,8 @@ struct Requirements {
std::optional<Capability::Capability> Cap = {},
ExtensionList Exts = {}, VersionTuple MinVer = VersionTuple(),
VersionTuple MaxVer = VersionTuple())
- : IsSatisfiable(IsSatisfiable), Cap(Cap), Exts(Exts), MinVer(MinVer),
- MaxVer(MaxVer) {}
+ : IsSatisfiable(IsSatisfiable), Cap(Cap), Exts(std::move(Exts)),
+ MinVer(MinVer), MaxVer(MaxVer) {}
Requirements(Capability::Capability Cap) : Requirements(true, {Cap}) {}
};
@@ -217,7 +217,8 @@ struct SPIRVModuleAnalysis : public ModulePass {
static char ID;
public:
- SPIRVModuleAnalysis() : ModulePass(ID) {}
+ SPIRVModuleAnalysis()
+ : ModulePass(ID), ST(nullptr), GR(nullptr), TII(nullptr), MMI(nullptr) {}
bool runOnModule(Module &M) override;
void getAnalysisUsage(AnalysisUsage &AU) const override;
diff --git a/llvm/lib/Target/SPIRV/SPIRVPostLegalizer.cpp b/llvm/lib/Target/SPIRV/SPIRVPostLegalizer.cpp
index 1d38244feeae7..d17528dd882bf 100644
--- a/llvm/lib/Target/SPIRV/SPIRVPostLegalizer.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVPostLegalizer.cpp
@@ -147,7 +147,7 @@ void visit(MachineFunction &MF, MachineBasicBlock &Start,
// Do a preorder traversal of the CFG starting from the given function's entry
// point. Calls |op| on each basic block encountered during the traversal.
void visit(MachineFunction &MF, std::function<void(MachineBasicBlock *)> op) {
- visit(MF, *MF.begin(), op);
+ visit(MF, *MF.begin(), std::move(op));
}
bool SPIRVPostLegalizer::runOnMachineFunction(MachineFunction &MF) {
diff --git a/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp b/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
index f4b4846f70d7d..b62db7fd62b2e 100644
--- a/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
@@ -99,6 +99,7 @@ addConstantsToTrack(MachineFunction &MF, SPIRVGlobalRegistry *GR,
SPIRVType *ExtType = GR->getOrCreateSPIRVType(
Const->getType(), MIB, SPIRV::AccessQualifier::ReadWrite,
true);
+ assert(SrcMI && "Expected source instruction to be valid");
SrcMI->setDesc(STI.getInstrInfo()->get(SPIRV::OpConstantNull));
SrcMI->addOperand(MachineOperand::CreateReg(
GR->getSPIRVTypeID(ExtType), false));
diff --git a/llvm/lib/Target/SPIRV/SPIRVPrepareFunctions.cpp b/llvm/lib/Target/SPIRV/SPIRVPrepareFunctions.cpp
index 595424b999439..74aec4fd9b358 100644
--- a/llvm/lib/Target/SPIRV/SPIRVPrepareFunctions.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVPrepareFunctions.cpp
@@ -234,7 +234,7 @@ static SmallVector<Metadata *> parseAnnotation(Value *I,
return SmallVector<Metadata *>{};
MDs.push_back(MDNode::get(Ctx, MDsItem));
}
- return Pos == static_cast<int>(Anno.length()) ? MDs
+ return Pos == static_cast<int>(Anno.length()) ? std::move(MDs)
: SmallVector<Metadata *>{};
}
diff --git a/llvm/lib/Target/SPIRV/SPIRVUtils.cpp b/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
index 416d811ba4e65..a6fbf82f9c65c 100644
--- a/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
@@ -463,8 +463,9 @@ std::string getOclOrSpirvBuiltinDemangledName(StringRef Name) {
DemangledNameLenStart = NameSpaceStart + 11;
}
Start = Name.find_first_not_of("0123456789", DemangledNameLenStart);
- Name.substr(DemangledNameLenStart, Start - DemangledNameLenStart)
+ bool Error = Name.substr(DemangledNameLenStart, Start - DemangledNameLenStart)
.getAsInteger(10, Len);
+ assert(!Error && "Failed to parse demangled name length");
return Name.substr(Start, Len).str();
}
@@ -756,7 +757,7 @@ bool getVacantFunctionName(Module &M, std::string &Name) {
for (unsigned I = 0; I < MaxIters; ++I) {
std::string OrdName = Name + Twine(I).str();
if (!M.getFunction(OrdName)) {
- Name = OrdName;
+ Name = std::move(OrdName);
return true;
}
}
>From 56224d9f5e431e73bb07d2d6246274eea9c89388 Mon Sep 17 00:00:00 2001
From: Marcos Maronas <marcos.maronas at intel.com>
Date: Mon, 4 Aug 2025 19:14:22 +0200
Subject: [PATCH 2/3] Fix clang-format issue.
---
llvm/lib/Target/SPIRV/SPIRVUtils.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Target/SPIRV/SPIRVUtils.cpp b/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
index a6fbf82f9c65c..08d6b1b5b059f 100644
--- a/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
@@ -464,7 +464,7 @@ std::string getOclOrSpirvBuiltinDemangledName(StringRef Name) {
}
Start = Name.find_first_not_of("0123456789", DemangledNameLenStart);
bool Error = Name.substr(DemangledNameLenStart, Start - DemangledNameLenStart)
- .getAsInteger(10, Len);
+ .getAsInteger(10, Len);
assert(!Error && "Failed to parse demangled name length");
return Name.substr(Start, Len).str();
}
>From 456c10c38bdfeb791c2284a5a0e0685de0179d03 Mon Sep 17 00:00:00 2001
From: Marcos Maronas <marcos.maronas at intel.com>
Date: Tue, 5 Aug 2025 17:08:33 +0200
Subject: [PATCH 3/3] Address code review feedback.
---
.../Analysis/SPIRVConvergenceRegionAnalysis.h | 5 ++--
llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp | 3 ++-
llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp | 1 -
llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp | 23 ++++++++-----------
llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp | 5 ++--
llvm/lib/Target/SPIRV/SPIRVUtils.cpp | 5 ++--
6 files changed, 19 insertions(+), 23 deletions(-)
diff --git a/llvm/lib/Target/SPIRV/Analysis/SPIRVConvergenceRegionAnalysis.h b/llvm/lib/Target/SPIRV/Analysis/SPIRVConvergenceRegionAnalysis.h
index ca4b8459e9f36..ed0a1e10562a8 100644
--- a/llvm/lib/Target/SPIRV/Analysis/SPIRVConvergenceRegionAnalysis.h
+++ b/llvm/lib/Target/SPIRV/Analysis/SPIRVConvergenceRegionAnalysis.h
@@ -73,7 +73,6 @@ class ConvergenceRegion {
Entry(std::move(CR.Entry)), Exits(std::move(CR.Exits)),
Blocks(std::move(CR.Blocks)) {}
- // Destructor.
~ConvergenceRegion() { releaseMemory(); }
ConvergenceRegion &operator=(ConvergenceRegion &&CR) = delete;
@@ -106,8 +105,8 @@ class ConvergenceRegionInfo {
~ConvergenceRegionInfo() { releaseMemory(); }
- ConvergenceRegionInfo(const ConvergenceRegionInfo &LHS) = default;
- ConvergenceRegionInfo &operator=(const ConvergenceRegionInfo &LHS) = default;
+ ConvergenceRegionInfo(const ConvergenceRegionInfo &LHS) = delete;
+ ConvergenceRegionInfo &operator=(const ConvergenceRegionInfo &LHS) = delete;
ConvergenceRegionInfo(ConvergenceRegionInfo &&LHS)
: TopLevelRegion(LHS.TopLevelRegion) {
diff --git a/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp b/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
index 2ada672156d6f..c2a6e51913a0a 100644
--- a/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
@@ -592,7 +592,8 @@ void SPIRVAsmPrinter::outputAnnotations(const Module &M) {
cast<GlobalVariable>(CS->getOperand(1)->stripPointerCasts());
StringRef AnnotationString;
- bool Success = getConstantStringInfo(GV, AnnotationString);
+ [[maybe_unused]] bool Success =
+ getConstantStringInfo(GV, AnnotationString);
assert(Success && "Failed to get annotation string");
MCInst Inst;
Inst.setOpcode(SPIRV::OpDecorate);
diff --git a/llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp b/llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp
index 6e2f477e80012..e6e86b71b2dcc 100644
--- a/llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp
@@ -3187,7 +3187,6 @@ static SPIRVType *getLayoutType(const TargetExtType *ExtensionType,
namespace SPIRV {
TargetExtType *parseBuiltinTypeNameToTargetExtType(std::string TypeName,
LLVMContext &Context) {
- assert(!TypeName.empty() && "Empty builtin type name!");
StringRef NameWithParameters = TypeName;
// Pointers-to-opaque-structs representing OpenCL types are first translated
diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index 239b55ae23e07..77ff6b0ece76d 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -893,19 +893,16 @@ Type *SPIRVEmitIntrinsics::deduceNestedTypeHelper(
bool Change = false;
for (unsigned i = 0; i < U->getNumOperands(); ++i) {
Value *Op = U->getOperand(i);
- Type *OpTy = nullptr;
- Type *Ty = nullptr;
- if (Op) {
- OpTy = Op->getType();
- Ty = OpTy;
- if (auto *PtrTy = dyn_cast<PointerType>(OpTy)) {
- if (Type *NestedTy =
- deduceElementTypeHelper(Op, Visited, UnknownElemTypeI8))
- Ty = getTypedPointerWrapper(NestedTy, PtrTy->getAddressSpace());
- } else {
- Ty = deduceNestedTypeHelper(dyn_cast<User>(Op), OpTy, Visited,
- UnknownElemTypeI8);
- }
+ assert(Op && "Operands should not be null.");
+ Type *OpTy = Op->getType();
+ Type *Ty = OpTy;
+ if (auto *PtrTy = dyn_cast<PointerType>(OpTy)) {
+ if (Type *NestedTy =
+ deduceElementTypeHelper(Op, Visited, UnknownElemTypeI8))
+ Ty = getTypedPointerWrapper(NestedTy, PtrTy->getAddressSpace());
+ } else {
+ Ty = deduceNestedTypeHelper(dyn_cast<User>(Op), OpTy, Visited,
+ UnknownElemTypeI8);
}
Tys.push_back(Ty);
Change |= Ty != OpTy;
diff --git a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
index a1b3a75d6706d..31b36efe30232 100644
--- a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
@@ -474,9 +474,8 @@ Register SPIRVGlobalRegistry::getOrCreateBaseRegister(
}
if (Type->getOpcode() == SPIRV::OpTypeFloat) {
SPIRVType *SpvBaseType = getOrCreateSPIRVFloatType(BitWidth, I, TII);
- assert(isa<ConstantFP>(Val) && "Expected ConstantFP for OpTypeFloat");
- return getOrCreateConstFP(dyn_cast<ConstantFP>(Val)->getValue(), I,
- SpvBaseType, TII, ZeroAsNull);
+ return getOrCreateConstFP(cast<ConstantFP>(Val)->getValue(), I, SpvBaseType,
+ TII, ZeroAsNull);
}
assert(Type->getOpcode() == SPIRV::OpTypeInt);
SPIRVType *SpvBaseType = getOrCreateSPIRVIntegerType(BitWidth, I, TII);
diff --git a/llvm/lib/Target/SPIRV/SPIRVUtils.cpp b/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
index 08d6b1b5b059f..820e56b362edc 100644
--- a/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
@@ -463,8 +463,9 @@ std::string getOclOrSpirvBuiltinDemangledName(StringRef Name) {
DemangledNameLenStart = NameSpaceStart + 11;
}
Start = Name.find_first_not_of("0123456789", DemangledNameLenStart);
- bool Error = Name.substr(DemangledNameLenStart, Start - DemangledNameLenStart)
- .getAsInteger(10, Len);
+ [[maybe_unused]] bool Error =
+ Name.substr(DemangledNameLenStart, Start - DemangledNameLenStart)
+ .getAsInteger(10, Len);
assert(!Error && "Failed to parse demangled name length");
return Name.substr(Start, Len).str();
}
More information about the llvm-commits
mailing list