[PATCH] D19498: [scan-build] fix logic error warnings emitted on llvm code base

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 25 15:03:10 PDT 2016


MatzeB added a comment.

I looked at the ScheduleDAG and ARM changes here.


================
Comment at: lib/CodeGen/ScheduleDAGInstrs.cpp:917
@@ -916,2 +916,3 @@
     MachineInstr *MI = std::prev(MII);
-    if (MI && DbgMI) {
+    assert(MI && "MachineInstr is null, cannot iterate through MachineBasicBlock");
+
----------------
I think this is so obvious that we don't even need the assert. We will get a crash in MI->isDebugValue() anyway when MI is nullptr.

================
Comment at: lib/Target/ARM/ARMFrameLowering.cpp:1710
@@ -1709,2 +1709,3 @@
         const TargetRegisterClass *RC = &ARM::GPRRegClass;
+        assert(RS && "is not initialized");
         RS->addScavengingFrameIndex(MFI->CreateStackObject(RC->getSize(),
----------------
ARMBaseRegisterInfo::requiresRegisterScavenging() always returns true. So this should never be nullptr, instead of adding the assert you can probably remove the nullptr-check in line 1610.

================
Comment at: lib/Target/ARM/Thumb2SizeReduction.cpp:976-977
@@ -975,2 +975,4 @@
 
+    assert(BundleMI && "MachineInstr is null, abort reducing width of instructions");
+
     LiveCPSR = UpdateCPSRUse(*MI, LiveCPSR);
----------------
This looks like it needs to go inside the "if (MI->isInsideBundle())" case below. I must admit I also don't understand why the analyzer would warn here, there is nothing obvious that indicates that BundleMI may be a nullptr below...


http://reviews.llvm.org/D19498





More information about the llvm-commits mailing list