[llvm] Fixes simple issue found static analyzer (PR #169958)

via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 28 12:50:35 PST 2025


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-globalisel

Author: None (Seraphimt)

<details>
<summary>Changes</summary>

Hi! I found several issues in LLVM code using static analyzer tool PVS-studio. I hope this will be helpful. 
Full descriptions in the article: https://pvs-studio.com/en/blog/posts/cpp/1318/
I took the liberty of correcting some simple fragments myself:
- [ ] 1. Misprint in conditional.
The PVS-Studio warning: V501 There are identical sub-expressions '!Subtarget.hasZeroCycleRegMoveFPR64()' to the left and to the right of the '&&' operator. AArch64InstrInfo.cpp 5430
- [ ] 3. Array overrun is possible.
The PVS-Studio warning: V557 Array overrun is possible. The value of 'regIdx' index could reach 31. VEAsmParser.cpp 696
- [ ] 10. Excessive check.
The PVS-Studio warning: V547 Expression 'IsLeaf' is always false. PPCInstrInfo.cpp 419
- [ ] 11. Doubling the same check.
The PVS-Studio warning: V581 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 5820, 5823. PPCInstrInfo.cpp 5823
- [ ] 12. Wrong value in conditional.
The PVS-Studio warning: V547 Expression 'Opcode == Instruction::FDiv' is always false. GISelValueTracking.cpp 1451
- [ ] 14. Type issue in expression.
The PVS-Studio warning: V560 A part of conditional expression is always false: AVX10Ver >= 2. Host.cpp 2177
- [ ] 15. Excessive check.
The PVS-Studio warning: V547 Expression 'i != e' is always true. MachineFunction.cpp 1444
- [ ] 17. Excessive assignment.
The PVS-Studio warning: V1048 The 'FirstOp' variable was assigned the same value. MachineInstr.cpp 1995
- [ ] 18. Excessive check.
The PVS-Studio warning: V547 Expression 'AllSame' is always true. SimplifyCFG.cpp 1914
- [ ] 19. Excessive check.
The PVS-Studio warning: V547 Expression 'AbbrevDecl' is always true. LVDWARFReader.cpp 398

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


9 Files Affected:

- (modified) llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp (+2-2) 
- (modified) llvm/lib/CodeGen/MachineFunction.cpp (+1-2) 
- (modified) llvm/lib/CodeGen/MachineInstr.cpp (-5) 
- (modified) llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp (+3-4) 
- (modified) llvm/lib/Target/AArch64/AArch64InstrInfo.cpp (+1-1) 
- (modified) llvm/lib/Target/PowerPC/PPCInstrInfo.cpp (+1-4) 
- (modified) llvm/lib/Target/VE/AsmParser/VEAsmParser.cpp (+1-1) 
- (modified) llvm/lib/TargetParser/Host.cpp (+3-3) 
- (modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+8-10) 


``````````diff
diff --git a/llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp b/llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp
index ecba323f8d6bf..125b5ad6d57d0 100644
--- a/llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp
@@ -1373,7 +1373,7 @@ void GISelValueTracking::computeKnownFPClass(Register R,
           (KnownLHS.isKnownNeverInfinity() || KnownRHS.isKnownNeverInfinity()))
         Known.knownNot(fcNan);
 
-      if (Opcode == Instruction::FAdd) {
+      if (Opcode == TargetOpcode::G_FADD) {
         if (KnownLHS.cannotBeOrderedLessThanZero() &&
             KnownRHS.cannotBeOrderedLessThanZero())
           Known.knownNot(KnownFPClass::OrderedLessThanZeroMask);
@@ -1488,7 +1488,7 @@ void GISelValueTracking::computeKnownFPClass(Register R,
                           KnownLHS, Depth + 1);
     }
 
-    if (Opcode == Instruction::FDiv) {
+    if (Opcode == TargetOpcode::G_FDIV) {
       // Only 0/0, Inf/Inf produce NaN.
       if (KnownLHS.isKnownNeverNaN() && KnownRHS.isKnownNeverNaN() &&
           (KnownLHS.isKnownNeverInfinity() ||
diff --git a/llvm/lib/CodeGen/MachineFunction.cpp b/llvm/lib/CodeGen/MachineFunction.cpp
index 634547ded992f..8d7694537e07d 100644
--- a/llvm/lib/CodeGen/MachineFunction.cpp
+++ b/llvm/lib/CodeGen/MachineFunction.cpp
@@ -1439,8 +1439,7 @@ void MachineJumpTableInfo::print(raw_ostream &OS) const {
     OS << printJumpTableEntryReference(i) << ':';
     for (const MachineBasicBlock *MBB : JumpTables[i].MBBs)
       OS << ' ' << printMBBReference(*MBB);
-    if (i != e)
-      OS << '\n';
+    OS << '\n';
   }
 
   OS << '\n';
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index 18111156efa4f..c801d3deafa0f 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -2011,7 +2011,6 @@ void MachineInstr::print(raw_ostream &OS, ModuleSlotTracker &MST,
   // operands.
   if (MCSymbol *PreInstrSymbol = getPreInstrSymbol()) {
     if (!FirstOp) {
-      FirstOp = false;
       OS << ',';
     }
     OS << " pre-instr-symbol ";
@@ -2019,7 +2018,6 @@ void MachineInstr::print(raw_ostream &OS, ModuleSlotTracker &MST,
   }
   if (MCSymbol *PostInstrSymbol = getPostInstrSymbol()) {
     if (!FirstOp) {
-      FirstOp = false;
       OS << ',';
     }
     OS << " post-instr-symbol ";
@@ -2027,7 +2025,6 @@ void MachineInstr::print(raw_ostream &OS, ModuleSlotTracker &MST,
   }
   if (MDNode *HeapAllocMarker = getHeapAllocMarker()) {
     if (!FirstOp) {
-      FirstOp = false;
       OS << ',';
     }
     OS << " heap-alloc-marker ";
@@ -2035,7 +2032,6 @@ void MachineInstr::print(raw_ostream &OS, ModuleSlotTracker &MST,
   }
   if (MDNode *PCSections = getPCSections()) {
     if (!FirstOp) {
-      FirstOp = false;
       OS << ',';
     }
     OS << " pcsections ";
@@ -2043,7 +2039,6 @@ void MachineInstr::print(raw_ostream &OS, ModuleSlotTracker &MST,
   }
   if (MDNode *MMRA = getMMRAMetadata()) {
     if (!FirstOp) {
-      FirstOp = false;
       OS << ',';
     }
     OS << " mmra ";
diff --git a/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp b/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp
index 3ba5061718144..772d821dcda81 100644
--- a/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp
+++ b/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp
@@ -395,10 +395,9 @@ LVScope *LVDWARFReader::processOneDie(const DWARFDie &InputDIE, LVScope *Parent,
       if (abbrCode) {
         if (const DWARFAbbreviationDeclaration *AbbrevDecl =
                 TheDIE.getAbbreviationDeclarationPtr())
-          if (AbbrevDecl)
-            for (const DWARFAbbreviationDeclaration::AttributeSpec &AttrSpec :
-                 AbbrevDecl->attributes())
-              processOneAttribute(TheDIE, &CurrentEndOffset, AttrSpec);
+          for (const DWARFAbbreviationDeclaration::AttributeSpec &AttrSpec :
+               AbbrevDecl->attributes())
+            processOneAttribute(TheDIE, &CurrentEndOffset, AttrSpec);
       }
     };
 
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index 904577b8233d5..fe1640e2bdd6a 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -5732,7 +5732,7 @@ void AArch64InstrInfo::copyPhysReg(MachineBasicBlock &MBB,
       AArch64::FPR8RegClass.contains(SrcReg)) {
     if (Subtarget.hasZeroCycleRegMoveFPR128() &&
         !Subtarget.hasZeroCycleRegMoveFPR64() &&
-        !Subtarget.hasZeroCycleRegMoveFPR64() && Subtarget.isNeonAvailable()) {
+        !Subtarget.hasZeroCycleRegMoveFPR32() && Subtarget.isNeonAvailable()) {
       MCRegister DestRegQ = RI.getMatchingSuperReg(DestReg, AArch64::bsub,
                                                    &AArch64::FPR128RegClass);
       MCRegister SrcRegQ = RI.getMatchingSuperReg(SrcReg, AArch64::bsub,
diff --git a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
index 366a7b6d0135a..94348608d6603 100644
--- a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
+++ b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
@@ -416,7 +416,7 @@ bool PPCInstrInfo::getFMAPatterns(MachineInstr &Root,
 
     // If this is not Leaf FMA Instr, its 'add' operand should only have one use
     // as this fma will be changed later.
-    return IsLeaf ? true : MRI->hasOneNonDBGUse(OpAdd.getReg());
+    return MRI->hasOneNonDBGUse(OpAdd.getReg());
   };
 
   int16_t AddOpIdx = -1;
@@ -5809,9 +5809,6 @@ bool PPCInstrInfo::getMemOperandWithOffsetWidth(
   if (!LdSt.getOperand(1).isImm() ||
       (!LdSt.getOperand(2).isReg() && !LdSt.getOperand(2).isFI()))
     return false;
-  if (!LdSt.getOperand(1).isImm() ||
-      (!LdSt.getOperand(2).isReg() && !LdSt.getOperand(2).isFI()))
-    return false;
 
   if (!LdSt.hasOneMemOperand())
     return false;
diff --git a/llvm/lib/Target/VE/AsmParser/VEAsmParser.cpp b/llvm/lib/Target/VE/AsmParser/VEAsmParser.cpp
index 9b47d237f0702..eceddd66e39d5 100644
--- a/llvm/lib/Target/VE/AsmParser/VEAsmParser.cpp
+++ b/llvm/lib/Target/VE/AsmParser/VEAsmParser.cpp
@@ -694,7 +694,7 @@ class VEOperand : public MCParsedAsmOperand {
     if (!ConstExpr)
       return false;
     unsigned regIdx = ConstExpr->getValue();
-    if (regIdx > 31 || MISCRegs[regIdx] == VE::NoRegister)
+    if (regIdx >= std::size(MISCRegs) || MISCRegs[regIdx] == VE::NoRegister)
       return false;
     Op.Kind = k_Register;
     Op.Reg.Reg = MISCRegs[regIdx];
diff --git a/llvm/lib/TargetParser/Host.cpp b/llvm/lib/TargetParser/Host.cpp
index cb793d60a286f..d4aed19b2cb68 100644
--- a/llvm/lib/TargetParser/Host.cpp
+++ b/llvm/lib/TargetParser/Host.cpp
@@ -2195,9 +2195,9 @@ StringMap<bool> sys::getHostCPUFeatures() {
   bool HasLeaf24 =
       MaxLevel >= 0x24 && !getX86CpuIDAndInfo(0x24, &EAX, &EBX, &ECX, &EDX);
 
-  int AVX10Ver = HasLeaf24 && (EBX & 0xff);
-  Features["avx10.1"] = HasAVX10 && AVX10Ver >= 1;
-  Features["avx10.2"] = HasAVX10 && AVX10Ver >= 2;
+  int AVX10Ver = EBX & 0xff;
+  Features["avx10.1"] = HasAVX10 && HasLeaf24 && AVX10Ver >= 1;
+  Features["avx10.2"] = HasAVX10 && HasLeaf24 && AVX10Ver >= 2;
 
   return Features;
 }
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index ed2a5c292fa54..3285147539501 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1909,17 +1909,15 @@ bool SimplifyCFGOpt::hoistCommonCodeFromSuccessors(Instruction *TI,
     });
     if (!AllSame)
       return false;
-    if (AllSame) {
-      LockstepReverseIterator<true> LRI(Succs);
-      while (LRI.isValid()) {
-        Instruction *I0 = (*LRI)[0];
-        if (any_of(*LRI, [I0](Instruction *I) {
-              return !areIdenticalUpToCommutativity(I0, I);
-            })) {
-          return false;
-        }
-        --LRI;
+    LockstepReverseIterator<true> LRI(Succs);
+    while (LRI.isValid()) {
+      Instruction *I0 = (*LRI)[0];
+      if (any_of(*LRI, [I0](Instruction *I) {
+            return !areIdenticalUpToCommutativity(I0, I);
+          })) {
+        return false;
       }
+      --LRI;
     }
     // Now we know that all instructions in all successors can be hoisted. Let
     // the loop below handle the hoisting.

``````````

</details>


https://github.com/llvm/llvm-project/pull/169958


More information about the llvm-commits mailing list