Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add and call AMDGPUMCResourceInfo::reset method #110818

Merged
merged 2 commits into from
Oct 2, 2024

Conversation

tsymalla
Copy link
Contributor

@tsymalla tsymalla commented Oct 2, 2024

When compiling multiple pipelines, the MCRegisterInfo instance in AMDGPUAsmPrinter gets re-used even after finalization, so it calls finalize() multiple times.

Add a reset method and call it in
AMDGPUAsmPrinter::doFinalization.

Different approach would be to make it a unique_ptr.

@llvmbot
Copy link
Collaborator

llvmbot commented Oct 2, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Thomas Symalla (tsymalla)

Changes

When compiling multiple pipelines, the MCRegisterInfo instance in AMDGPUAsmPrinter gets re-used even after finalization, so it calls finalize() multiple times.

Add a reset method and call it in
AMDGPUAsmPrinter::doInitialization.

Different approach would be to make it a unique_ptr.


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

3 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp (+3)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPUMCResourceInfo.cpp (+7)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPUMCResourceInfo.h (+2)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp b/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
index 4f6633d8027c70..8cc2132a2e6fe9 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
@@ -358,6 +358,9 @@ bool AMDGPUAsmPrinter::doInitialization(Module &M) {
       report_fatal_error("Unexpected code object version");
     }
   }
+
+  RI.reset();
+  
   return AsmPrinter::doInitialization(M);
 }
 
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUMCResourceInfo.cpp b/llvm/lib/Target/AMDGPU/AMDGPUMCResourceInfo.cpp
index f608a9a4f470fa..8293c998c05c76 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUMCResourceInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUMCResourceInfo.cpp
@@ -71,6 +71,13 @@ void MCResourceInfo::assignMaxRegs(MCContext &OutContext) {
   assignMaxRegSym(MaxSGPRSym, MaxSGPR);
 }
 
+void MCResourceInfo::reset() {
+  Finalized = false;
+  MaxVGPR = 0;
+  MaxAGPR = 0;
+  MaxSGPR = 0;
+}
+
 void MCResourceInfo::finalize(MCContext &OutContext) {
   assert(!Finalized && "Cannot finalize ResourceInfo again.");
   Finalized = true;
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUMCResourceInfo.h b/llvm/lib/Target/AMDGPU/AMDGPUMCResourceInfo.h
index 08c0c106d5aa9b..9f7f4b1b8f75ff 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUMCResourceInfo.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPUMCResourceInfo.h
@@ -75,6 +75,8 @@ class MCResourceInfo {
   const MCExpr *getSymRefExpr(StringRef FuncName, ResourceInfoKind RIK,
                               MCContext &Ctx);
 
+  void reset();
+  
   // Resolves the final symbols that requires the inter-function resource info
   // to be resolved.
   void finalize(MCContext &OutContext);

@jasilvanus
Copy link
Contributor

Why not wrap it into an std::optional?

Copy link

github-actions bot commented Oct 2, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@tsymalla
Copy link
Contributor Author

tsymalla commented Oct 2, 2024

Why not wrap it into an std::optional?

It would work, but I didn't want to introduce another way of defining members to the AsmPrinter :-)

When compiling multiple pipelines, the `MCRegisterInfo` instance in
`AMDGPUAsmPrinter` gets re-used even after finalization, so it calls
`finalize()` multiple times.

Add a reset method and call it in
`AMDGPUAsmPrinter::doFinalization`.
@tsymalla tsymalla merged commit b95d50e into llvm:main Oct 2, 2024
8 checks passed
Sterling-Augustine pushed a commit to Sterling-Augustine/llvm-project that referenced this pull request Oct 3, 2024
When compiling multiple pipelines, the `MCRegisterInfo` instance in
`AMDGPUAsmPrinter` gets re-used even after finalization, so it calls
`finalize()` multiple times.

Add a reset method and call it in
`AMDGPUAsmPrinter::doFinalization`.

Different approach would be to make it a `unique_ptr`.

---------

Co-authored-by: Thomas Symalla <tsymalla@amd.com>
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.

4 participants