Skip to content

[NFC][RA] Refactor RABasic into a Separate Header #149555

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kyulee-com
Copy link
Contributor

This change refactors the RABasic type by moving it from RegAllocBasic.cpp to a new header file, RegAllocBasic.h. This separation of header and implementation aligns with the structure used by other register allocators, such as RegAllocGreedy. The refactoring is intended to facilitate future use of RABasic in other contexts.

This change refactors the RABasic type by moving it from RegAllocBasic.cpp to a new header file, RegAllocBasic.h. This separation of header and implementation aligns with the structure used by other register allocators, such as RegAllocGreedy. The refactoring is intended to facilitate future use of RABasic in other contexts.
Copy link
Contributor

@MatzeB MatzeB left a comment

Choose a reason for hiding this comment

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

In principle I'm fine with this. What's the uses you have in mind? Will it be used upstream?

Comment on lines 9 to 10
// This file declares the RABasic class, which provides a minimal
// implementation of the basic register allocator.
Copy link
Contributor

Choose a reason for hiding this comment

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

use syntax recognized by doxygen (and possibly fix the pre-existing problem in RegAllocBasic.cpp)

Suggested change
// This file declares the RABasic class, which provides a minimal
// implementation of the basic register allocator.
/// \file This file declares the RABasic class, which provides a minimal
/// implementation of the basic register allocator.

Comment on lines 14 to 15
#ifndef LLVM_CODEGEN_REGALLOCBAISC_H_
#define LLVM_CODEGEN_REGALLOCBAISC_H_
Copy link
Contributor

@MatzeB MatzeB Jul 18, 2025

Choose a reason for hiding this comment

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

Suggested change
#ifndef LLVM_CODEGEN_REGALLOCBAISC_H_
#define LLVM_CODEGEN_REGALLOCBAISC_H_
#ifndef LLVM_CODEGEN_REGALLOCBASIC_H
#define LLVM_CODEGEN_REGALLOCBASIC_H

Copy link
Contributor

Choose a reason for hiding this comment

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

most files tend to use LLVM_LIB_CODEGEN_XXXX_H it seems...

static char ID;
};
} // namespace llvm
#endif // #ifndef LLVM_CODEGEN_REGALLOCBAISC_H_
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#endif // #ifndef LLVM_CODEGEN_REGALLOCBAISC_H_
#endif

Comment on lines 19 to 24
#include "llvm/CodeGen/LiveIntervals.h"
#include "llvm/CodeGen/LiveRangeEdit.h"
#include "llvm/CodeGen/LiveRegMatrix.h"
#include "llvm/CodeGen/MachineFunctionPass.h"
#include "llvm/CodeGen/Spiller.h"
#include "llvm/CodeGen/VirtRegMap.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please check which include files are actually needed, and which ones can be replaced with class xxx; forward declarations. LiveRegMatrix and VirtRegMap seem unused at a first glance...

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