Skip to content

release/22.x: [Hexagon] Define __HVX_IEEE_FP__ ; Use __HVX_IEEE_FP__ to guard protos #185014

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

Open
androm3da wants to merge 2 commits into
base: release/22.x
Choose a base branch
from
Open

release/22.x: [Hexagon] Define __HVX_IEEE_FP__ ; Use __HVX_IEEE_FP__ to guard protos #185014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
de9b26f
[Hexagon] Define __HVX_IEEE_FP__ when -mhvx-ieee-fp is enabled (#183829)
aankit-ca Feb 27, 2026
284abb6
[Hexagon] Use __HVX_IEEE_FP__ to guard protos that need -mhvx-ieee-fp…
quic-akaryaki Mar 4, 2026
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
6 changes: 6 additions & 0 deletions clang/lib/Basic/Targets/Hexagon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ void HexagonTargetInfo::getTargetDefines(const LangOptions &Opts,
Builder.defineMacro("__HVXDBL__");
}

if (HasHVXIeeeFp)
Builder.defineMacro("__HVX_IEEE_FP__");

if (hasFeature("audio")) {
Builder.defineMacro("__HEXAGON_AUDIO__");
}
Expand Down Expand Up @@ -148,6 +151,8 @@ bool HexagonTargetInfo::handleTargetFeatures(std::vector<std::string> &Features,
UseLongCalls = true;
else if (F == "-long-calls")
UseLongCalls = false;
else if (F == "+hvx-ieee-fp")
HasHVXIeeeFp = true;
else if (F == "+audio")
HasAudio = true;
}
Expand Down Expand Up @@ -242,6 +247,7 @@ bool HexagonTargetInfo::hasFeature(StringRef Feature) const {
.Case("hvx", HasHVX)
.Case("hvx-length64b", HasHVX64B)
.Case("hvx-length128b", HasHVX128B)
.Case("hvx-ieee-fp", HasHVXIeeeFp)
Copy link
Copy Markdown
Collaborator

@AaronBallman AaronBallman Mar 27, 2026

Choose a reason for hiding this comment

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

Is this backporting a new feature or is it backporting work already in progress on 22.x?

Copy link
Copy Markdown
Member Author

@androm3da androm3da Mar 28, 2026

Choose a reason for hiding this comment

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

The hvx-ieee-fp feature was introduced in f766bc0 which was available in LLVM 14.0.0.

Copy link
Copy Markdown
Collaborator

@AaronBallman AaronBallman Mar 30, 2026

Choose a reason for hiding this comment

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

Thanks!

.Case("long-calls", UseLongCalls)
.Case("audio", HasAudio)
.Default(false);
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Basic/Targets/Hexagon.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class LLVM_LIBRARY_VISIBILITY HexagonTargetInfo : public TargetInfo {
bool HasHVX = false;
bool HasHVX64B = false;
bool HasHVX128B = false;
bool HasHVXIeeeFp = false;
Copy link
Copy Markdown
Collaborator

@AaronBallman AaronBallman Mar 27, 2026

Choose a reason for hiding this comment

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

I believe this changes the ABI of the HexagonTargetInfo class which is exported with library visibility.

Copy link
Copy Markdown
Member

@mstorsjo mstorsjo Mar 27, 2026

Choose a reason for hiding this comment

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

This class isn't part of the public interface of libclang-cpp, and "library visibility" means "visible only inside the library"; see https://github.com/llvm/llvm-project/blob/llvmorg-22.1.2/llvm/include/llvm/Support/Compiler.h#L107-L154:

LLVM_LIBRARY_VISIBILITY - If a class marked with this attribute is linked into a shared library, then the class should be private to the library and not accessible from outside it.

Copy link
Copy Markdown
Collaborator

@AaronBallman AaronBallman Mar 27, 2026

Choose a reason for hiding this comment

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

Whoops, thank you for clarifying! It does change the class layout, but not at an expected ABI boundary.

bool HasAudio = false;
bool UseLongCalls = false;

Expand Down
Loading
Loading