Skip to content

[AMDGPU] Coverity fixes - check ret val and init class members#198570

Merged
arsenm merged 1 commit into
llvm:mainfrom
bratpiorka:rrudnick_amd_coverity_fixes2
May 21, 2026
Merged

[AMDGPU] Coverity fixes - check ret val and init class members#198570
arsenm merged 1 commit into
llvm:mainfrom
bratpiorka:rrudnick_amd_coverity_fixes2

Conversation

@bratpiorka
Copy link
Copy Markdown
Contributor

@bratpiorka bratpiorka commented May 19, 2026

Coverity fixes:

  • calling getIntrinsicSignature without checking return value (as is done elsewhere 4 out of 5 times) in llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
  • non-static class member MaxSGPRs, MaxVGPRs and MaxUnifiedVGPRs is not initialized in this constructor nor in any functions that it calls in llvm/lib/Target/AMDGPU/GCNRegPressure.h

@llvmorg-github-actions
Copy link
Copy Markdown

@llvm/pr-subscribers-backend-amdgpu

Author: Rafał Rudnicki (bratpiorka)

Changes

Coverity fixes


Full diff: https://github.com/llvm/llvm-project/pull/198570.diff

2 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp (+3-2)
  • (modified) llvm/lib/Target/AMDGPU/GCNRegPressure.h (+1-1)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp b/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
index 6bf70686f5569..b2aed7c5d12c3 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
@@ -279,9 +279,10 @@ simplifyAMDGCNImageIntrinsic(const GCNSubtarget *ST,
         // Obtain the original image sample intrinsic's signature
         // and replace its return type with the half-vector for D16 folding
         SmallVector<Type *, 8> SigTys;
-        Intrinsic::getIntrinsicSignature(II.getCalledFunction(), SigTys);
-        SigTys[0] = HalfVecTy;
+        if (!Intrinsic::getIntrinsicSignature(II.getCalledFunction(), SigTys))
+          return std::nullopt;
 
+        SigTys[0] = HalfVecTy;
         Module *M = II.getModule();
         Function *HalfDecl =
             Intrinsic::getOrInsertDeclaration(M, ImageDimIntr->Intr, SigTys);
diff --git a/llvm/lib/Target/AMDGPU/GCNRegPressure.h b/llvm/lib/Target/AMDGPU/GCNRegPressure.h
index c55796c37f287..c53e4a8a6313e 100644
--- a/llvm/lib/Target/AMDGPU/GCNRegPressure.h
+++ b/llvm/lib/Target/AMDGPU/GCNRegPressure.h
@@ -296,7 +296,7 @@ class GCNRPTarget {
 
   GCNRPTarget(const GCNRegPressure &RP, const MachineFunction &MF)
       : MF(MF), UnifiedRF(MF.getSubtarget<GCNSubtarget>().hasGFX90AInsts()),
-        RP(RP) {}
+        RP(RP), MaxSGPRs(0), MaxVGPRs(0), MaxUnifiedVGPRs(0) {}
 };
 
 ///////////////////////////////////////////////////////////////////////////////

@shiltian
Copy link
Copy Markdown
Contributor

can you be more specific in the title and description?

@bratpiorka bratpiorka force-pushed the rrudnick_amd_coverity_fixes2 branch from bfd62f4 to ba3b338 Compare May 19, 2026 16:33
@bratpiorka
Copy link
Copy Markdown
Contributor Author

@shiltian done

@bratpiorka bratpiorka changed the title [AMDGPU] Coverity fixes [AMDGPU] Coverity fixes - check ret val and init class members May 19, 2026

GCNRPTarget(const GCNRegPressure &RP, const MachineFunction &MF)
: MF(MF), UnifiedRF(MF.getSubtarget<GCNSubtarget>().hasGFX90AInsts()),
RP(RP) {}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I'd prefer this to be something like:

  /// Target number of SGPRs.
  unsigned MaxSGPRs = 0;
  /// Target number of ArchVGPRs and AGPRs.
  unsigned MaxVGPRs = 0;
  /// Target number of overall VGPRs for subtargets with unified RFs. Always 0
  /// for subtargets with non-unified RFs.
  unsigned MaxUnifiedVGPRs = 0;

Copy link
Copy Markdown
Contributor

@shiltian shiltian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a nit

@bratpiorka bratpiorka force-pushed the rrudnick_amd_coverity_fixes2 branch from ba3b338 to 0ae9f8d Compare May 19, 2026 16:42
@bratpiorka
Copy link
Copy Markdown
Contributor Author

@shiltian ok, done

@bratpiorka
Copy link
Copy Markdown
Contributor Author

@shiltian all tests passed - please merge

@arsenm arsenm merged commit 1c305a3 into llvm:main May 21, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants