[SPIRV] Fix code quality issues. #152005
Open
[SPIRV] Fix code quality issues. #152005
+47
−24
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-backend-spir-v Author: Marcos Maronas (maarquitos14) ChangesFix code quality issues reported by static analysis tool, such as:
Full diff: https://github.com/llvm/llvm-project/pull/152005.diff 15 Files Affected:
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;
}
}
|
|
@MrSidims @Keenuts @michalpaszkowski please, take a look :) |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix code quality issues reported by static analysis tool, such as: