This is an archive of the discontinued LLVM Phabricator instance.

Fix uninitialized pointer members in CodeGen
ClosedPublic

Authored by akshaykhadse on Apr 13 2023, 10:50 PM.

Details

Summary

This change initializes the members TSI, LI, DT, PSI, and ORE pointer feilds of the SelectOptimize class to nullptr.

Diff Detail

Event Timeline

akshaykhadse created this revision.Apr 13 2023, 10:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2023, 10:50 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
akshaykhadse requested review of this revision.Apr 13 2023, 10:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2023, 10:50 PM
This revision is now accepted and ready to land.Apr 14 2023, 12:11 AM

Add more files with similar changes

Separte patch for Target files

Add similar fixes from other CodeGen files

Fix clang-format errors

akshaykhadse retitled this revision from Fix uninitialized non-static pointer members to Fix uninitialized pointer members in CodeGen.Apr 14 2023, 9:15 PM

Is this fixing a functional issue or some static analysis error?

Is this fixing a functional issue or some static analysis error?

These are fixing static analysis errors.

The premerge clang-format check complains about the file llvm/lib/CodeGen/RegAllocBasic.cpp and hence the build fails.

Upon investigation I found that it expects the following unrelated formatting changes to be present:

diff --git a/llvm/lib/CodeGen/RegAllocBasic.cpp b/llvm/lib/CodeGen/RegAllocBasic.cpp
index bef95e235b16..77d2eaabde72 100644
--- a/llvm/lib/CodeGen/RegAllocBasic.cpp
+++ b/llvm/lib/CodeGen/RegAllocBasic.cpp
@@ -41,12 +41,12 @@ static RegisterRegAlloc basicRegAlloc("basic", "basic register allocator",
                                       createBasicRegisterAllocator);
 
 namespace {
-  struct CompSpillWeight {
-    bool operator()(const LiveInterval *A, const LiveInterval *B) const {
-      return A->weight() < B->weight();
-    }
-  };
-}
+struct CompSpillWeight {
+  bool operator()(const LiveInterval *A, const LiveInterval *B) const {
+    return A->weight() < B->weight();
+  }
+};
+} // namespace
 
 namespace {
 /// RABasic provides a minimal implementation of the basic register allocation
@@ -109,7 +109,7 @@ public:
 
   MachineFunctionProperties getClearedProperties() const override {
     return MachineFunctionProperties().set(
-      MachineFunctionProperties::Property::IsSSA);
+        MachineFunctionProperties::Property::IsSSA);
   }
 
   // Helper for spilling all live virtual registers currently unified under preg
@@ -168,10 +168,8 @@ void RABasic::LRE_WillShrinkVirtReg(Register VirtReg) {
   enqueue(&LI);
 }
 
-RABasic::RABasic(RegClassFilterFunc F):
-  MachineFunctionPass(ID),
-  RegAllocBase(F) {
-}
+RABasic::RABasic(RegClassFilterFunc F)
+    : MachineFunctionPass(ID), RegAllocBase(F) {}
 
 void RABasic::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.setPreservesCFG();
@@ -197,10 +195,7 @@ void RABasic::getAnalysisUsage(AnalysisUsage &AU) const {
   MachineFunctionPass::getAnalysisUsage(AU);
 }
 
-void RABasic::releaseMemory() {
-  SpillerInstance.reset();
-}
-
+void RABasic::releaseMemory() { SpillerInstance.reset(); }
 
 // Spill or split all live virtual registers currently unified under PhysReg
 // that interfere with VirtReg. The newly spilled or split live intervals are
@@ -311,8 +306,7 @@ bool RABasic::runOnMachineFunction(MachineFunction &mf) {
                     << "********** Function: " << mf.getName() << '\n');
 
   MF = &mf;
-  RegAllocBase::init(getAnalysis<VirtRegMap>(),
-                     getAnalysis<LiveIntervals>(),
+  RegAllocBase::init(getAnalysis<VirtRegMap>(), getAnalysis<LiveIntervals>(),
                      getAnalysis<LiveRegMatrix>());
   VirtRegAuxInfo VRAI(*MF, *LIS, *VRM, getAnalysis<MachineLoopInfo>(),
                       getAnalysis<MachineBlockFrequencyInfo>());
@@ -330,10 +324,8 @@ bool RABasic::runOnMachineFunction(MachineFunction &mf) {
   return true;
 }
 
-FunctionPass* llvm::createBasicRegisterAllocator() {
-  return new RABasic();
-}
+FunctionPass *llvm::createBasicRegisterAllocator() { return new RABasic(); }
 
-FunctionPass* llvm::createBasicRegisterAllocator(RegClassFilterFunc F) {
+FunctionPass *llvm::createBasicRegisterAllocator(RegClassFilterFunc F) {
   return new RABasic(F);
 }

Should I add these even though they are not related?

The premerge clang-format check complains about the file llvm/lib/CodeGen/RegAllocBasic.cpp and hence the build fails.

Upon investigation I found that it expects the following unrelated formatting changes to be present:

diff --git a/llvm/lib/CodeGen/RegAllocBasic.cpp b/llvm/lib/CodeGen/RegAllocBasic.cpp
index bef95e235b16..77d2eaabde72 100644
--- a/llvm/lib/CodeGen/RegAllocBasic.cpp
+++ b/llvm/lib/CodeGen/RegAllocBasic.cpp
@@ -41,12 +41,12 @@ static RegisterRegAlloc basicRegAlloc("basic", "basic register allocator",
                                       createBasicRegisterAllocator);
 
 namespace {
-  struct CompSpillWeight {
-    bool operator()(const LiveInterval *A, const LiveInterval *B) const {
-      return A->weight() < B->weight();
-    }
-  };
-}
+struct CompSpillWeight {
+  bool operator()(const LiveInterval *A, const LiveInterval *B) const {
+    return A->weight() < B->weight();
+  }
+};
+} // namespace
 
 namespace {
 /// RABasic provides a minimal implementation of the basic register allocation
@@ -109,7 +109,7 @@ public:
 
   MachineFunctionProperties getClearedProperties() const override {
     return MachineFunctionProperties().set(
-      MachineFunctionProperties::Property::IsSSA);
+        MachineFunctionProperties::Property::IsSSA);
   }
 
   // Helper for spilling all live virtual registers currently unified under preg
@@ -168,10 +168,8 @@ void RABasic::LRE_WillShrinkVirtReg(Register VirtReg) {
   enqueue(&LI);
 }
 
-RABasic::RABasic(RegClassFilterFunc F):
-  MachineFunctionPass(ID),
-  RegAllocBase(F) {
-}
+RABasic::RABasic(RegClassFilterFunc F)
+    : MachineFunctionPass(ID), RegAllocBase(F) {}
 
 void RABasic::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.setPreservesCFG();
@@ -197,10 +195,7 @@ void RABasic::getAnalysisUsage(AnalysisUsage &AU) const {
   MachineFunctionPass::getAnalysisUsage(AU);
 }
 
-void RABasic::releaseMemory() {
-  SpillerInstance.reset();
-}
-
+void RABasic::releaseMemory() { SpillerInstance.reset(); }
 
 // Spill or split all live virtual registers currently unified under PhysReg
 // that interfere with VirtReg. The newly spilled or split live intervals are
@@ -311,8 +306,7 @@ bool RABasic::runOnMachineFunction(MachineFunction &mf) {
                     << "********** Function: " << mf.getName() << '\n');
 
   MF = &mf;
-  RegAllocBase::init(getAnalysis<VirtRegMap>(),
-                     getAnalysis<LiveIntervals>(),
+  RegAllocBase::init(getAnalysis<VirtRegMap>(), getAnalysis<LiveIntervals>(),
                      getAnalysis<LiveRegMatrix>());
   VirtRegAuxInfo VRAI(*MF, *LIS, *VRM, getAnalysis<MachineLoopInfo>(),
                       getAnalysis<MachineBlockFrequencyInfo>());
@@ -330,10 +324,8 @@ bool RABasic::runOnMachineFunction(MachineFunction &mf) {
   return true;
 }
 
-FunctionPass* llvm::createBasicRegisterAllocator() {
-  return new RABasic();
-}
+FunctionPass *llvm::createBasicRegisterAllocator() { return new RABasic(); }
 
-FunctionPass* llvm::createBasicRegisterAllocator(RegClassFilterFunc F) {
+FunctionPass *llvm::createBasicRegisterAllocator(RegClassFilterFunc F) {
   return new RABasic(F);
 }

Should I add these even though they are not related?

No don’t make unrelated formatting changes.

The premerge clang-format check complains about the file llvm/lib/CodeGen/RegAllocBasic.cpp and hence the build fails.

Upon investigation I found that it expects the following unrelated formatting changes to be present:

diff --git a/llvm/lib/CodeGen/RegAllocBasic.cpp b/llvm/lib/CodeGen/RegAllocBasic.cpp
index bef95e235b16..77d2eaabde72 100644
--- a/llvm/lib/CodeGen/RegAllocBasic.cpp
+++ b/llvm/lib/CodeGen/RegAllocBasic.cpp
@@ -41,12 +41,12 @@ static RegisterRegAlloc basicRegAlloc("basic", "basic register allocator",
                                       createBasicRegisterAllocator);
 
 namespace {
-  struct CompSpillWeight {
-    bool operator()(const LiveInterval *A, const LiveInterval *B) const {
-      return A->weight() < B->weight();
-    }
-  };
-}
+struct CompSpillWeight {
+  bool operator()(const LiveInterval *A, const LiveInterval *B) const {
+    return A->weight() < B->weight();
+  }
+};
+} // namespace
 
 namespace {
 /// RABasic provides a minimal implementation of the basic register allocation
@@ -109,7 +109,7 @@ public:
 
   MachineFunctionProperties getClearedProperties() const override {
     return MachineFunctionProperties().set(
-      MachineFunctionProperties::Property::IsSSA);
+        MachineFunctionProperties::Property::IsSSA);
   }
 
   // Helper for spilling all live virtual registers currently unified under preg
@@ -168,10 +168,8 @@ void RABasic::LRE_WillShrinkVirtReg(Register VirtReg) {
   enqueue(&LI);
 }
 
-RABasic::RABasic(RegClassFilterFunc F):
-  MachineFunctionPass(ID),
-  RegAllocBase(F) {
-}
+RABasic::RABasic(RegClassFilterFunc F)
+    : MachineFunctionPass(ID), RegAllocBase(F) {}
 
 void RABasic::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.setPreservesCFG();
@@ -197,10 +195,7 @@ void RABasic::getAnalysisUsage(AnalysisUsage &AU) const {
   MachineFunctionPass::getAnalysisUsage(AU);
 }
 
-void RABasic::releaseMemory() {
-  SpillerInstance.reset();
-}
-
+void RABasic::releaseMemory() { SpillerInstance.reset(); }
 
 // Spill or split all live virtual registers currently unified under PhysReg
 // that interfere with VirtReg. The newly spilled or split live intervals are
@@ -311,8 +306,7 @@ bool RABasic::runOnMachineFunction(MachineFunction &mf) {
                     << "********** Function: " << mf.getName() << '\n');
 
   MF = &mf;
-  RegAllocBase::init(getAnalysis<VirtRegMap>(),
-                     getAnalysis<LiveIntervals>(),
+  RegAllocBase::init(getAnalysis<VirtRegMap>(), getAnalysis<LiveIntervals>(),
                      getAnalysis<LiveRegMatrix>());
   VirtRegAuxInfo VRAI(*MF, *LIS, *VRM, getAnalysis<MachineLoopInfo>(),
                       getAnalysis<MachineBlockFrequencyInfo>());
@@ -330,10 +324,8 @@ bool RABasic::runOnMachineFunction(MachineFunction &mf) {
   return true;
 }
 
-FunctionPass* llvm::createBasicRegisterAllocator() {
-  return new RABasic();
-}
+FunctionPass *llvm::createBasicRegisterAllocator() { return new RABasic(); }
 
-FunctionPass* llvm::createBasicRegisterAllocator(RegClassFilterFunc F) {
+FunctionPass *llvm::createBasicRegisterAllocator(RegClassFilterFunc F) {
   return new RABasic(F);
 }

Should I add these even though they are not related?

No don’t make unrelated formatting changes.

Can we somehow make the build pass in this case?

Add related files

Remove files that require clang-format changes

@LuoYuanke Can you help me land this patch?

@LuoYuanke Can you help me land this patch?

Sure.

This revision was automatically updated to reflect the committed changes.