Skip to content

[RISCV] Don't expose any constructors of RISCVISAInfo publicly. #98249

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 10, 2024
Merged

[RISCV] Don't expose any constructors of RISCVISAInfo publicly. #98249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions lld/ELF/Arch/RISCV.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1251,9 +1251,8 @@ mergeAttributesSection(const SmallVector<InputSectionBase *, 0> &sections) {
}
}

if (hasArch) {
if (auto result = RISCVISAInfo::postProcessAndChecking(
std::make_unique<RISCVISAInfo>(xlen, exts))) {
if (hasArch && xlen != 0) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note, I had to add this xlen check since the xlen is 0 if we never successfully parsed an ISA string from any object. In that case we would have generated an error earlier so the linking would fail. Maybe it would be to have a flag that indicates we found an error earlier?

Copy link
Contributor

Choose a reason for hiding this comment

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

It could be useful to errorOrWarn after mergeArch call.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The errorOrWarn is already inside mergeArch

if (auto result = RISCVISAInfo::createFromExtMap(xlen, exts)) {
merged.strAttr.try_emplace(RISCVAttrs::ARCH,
saver().save((*result)->toString()));
} else {
Expand Down
12 changes: 7 additions & 5 deletions llvm/include/llvm/TargetParser/RISCVISAInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ class RISCVISAInfo {
RISCVISAInfo(const RISCVISAInfo &) = delete;
RISCVISAInfo &operator=(const RISCVISAInfo &) = delete;

RISCVISAInfo(unsigned XLen, RISCVISAUtils::OrderedExtensionMap &Exts)
: XLen(XLen), FLen(0), MinVLen(0), MaxELen(0), MaxELenFp(0), Exts(Exts) {}

/// Parse RISC-V ISA info from arch string.
/// If IgnoreUnknown is set, any unrecognised extension names or
/// extensions with unrecognised versions will be silently dropped, except
Expand All @@ -48,6 +45,10 @@ class RISCVISAInfo {
static llvm::Expected<std::unique_ptr<RISCVISAInfo>>
parseFeatures(unsigned XLen, const std::vector<std::string> &Features);

static llvm::Expected<std::unique_ptr<RISCVISAInfo>>
createFromExtMap(unsigned XLen,
const RISCVISAUtils::OrderedExtensionMap &Exts);

/// Convert RISC-V ISA info to a feature vector.
std::vector<std::string> toFeatures(bool AddAllExtensions = false,
bool IgnoreUnknown = true) const;
Expand All @@ -72,8 +73,6 @@ class RISCVISAInfo {
static bool isSupportedExtensionWithVersion(StringRef Ext);
static bool isSupportedExtension(StringRef Ext, unsigned MajorVersion,
unsigned MinorVersion);
static llvm::Expected<std::unique_ptr<RISCVISAInfo>>
postProcessAndChecking(std::unique_ptr<RISCVISAInfo> &&ISAInfo);
static std::string getTargetFeatureForExtension(StringRef Ext);

private:
Expand All @@ -93,6 +92,9 @@ class RISCVISAInfo {

/// Update FLen, MinVLen, MaxELen, and MaxELenFp.
void updateImpliedLengths();

static llvm::Expected<std::unique_ptr<RISCVISAInfo>>
postProcessAndChecking(std::unique_ptr<RISCVISAInfo> &&ISAInfo);
};

} // namespace llvm
Expand Down
11 changes: 11 additions & 0 deletions llvm/lib/TargetParser/RISCVISAInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,17 @@ static Error getExtensionVersion(StringRef Ext, StringRef In, unsigned &Major,
return getError(Error);
}

llvm::Expected<std::unique_ptr<RISCVISAInfo>>
RISCVISAInfo::createFromExtMap(unsigned XLen,
const RISCVISAUtils::OrderedExtensionMap &Exts) {
assert(XLen == 32 || XLen == 64);
std::unique_ptr<RISCVISAInfo> ISAInfo(new RISCVISAInfo(XLen));

ISAInfo->Exts = Exts;

return RISCVISAInfo::postProcessAndChecking(std::move(ISAInfo));
}

llvm::Expected<std::unique_ptr<RISCVISAInfo>>
RISCVISAInfo::parseFeatures(unsigned XLen,
const std::vector<std::string> &Features) {
Expand Down
Loading