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

RISC-V compiler improvements and CI #274

Merged
merged 15 commits into from
Nov 12, 2024
Merged

Conversation

karihepola
Copy link
Contributor

This PR adds the automatically retargeting compiler features presented at NorCAS 2024. Additionally, it adds the long-awaited CI pipeline.

@karihepola karihepola requested a review from pjaaskel November 11, 2024 09:24
Copy link
Contributor

@pjaaskel pjaaskel left a comment

Choose a reason for hiding this comment

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

Looks good. Small things. Great to finally have the CI.

- RISC-V compiler now generates new LLVM Targets from RISC-V ADFs
and loads them dynamically, which adds support for automatic
instruction selection for RISC-V custom instructions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps add a link to the paper.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The paper is not out yet. Perhaps we can add the link later when it is available.

}

std::string
RISCVTDGen::intToHexString(int num) const {
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we have this in StringTools? If not, they belong there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added to TCEString

// TODO: OpenASIP converts hex numbers to unsigned by default. This converted
// number might not fit into i32
std::string
RISCVTDGen::decimalsToHex(const std::string& pattern) const {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same. Belongs to StringTools.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would keep this here, since it is highly specific to the TDGen pattern that it operates on

Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/
/**
* @file riscv-tdgen
Copy link
Contributor

Choose a reason for hiding this comment

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

.cc

@@ -40,13 +40,13 @@ function fetch_llvm {

if ! test -d $llvm_co_dir;
then
git clone --single-branch --depth=1 --branch release/17.x https://github.com/cpc/llvmtce.git $llvm_co_dir\
git clone --single-branch --depth=1 --branch release/18.x https://github.com/cpc/llvmtce.git $llvm_co_dir\
Copy link
Contributor

Choose a reason for hiding this comment

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

Why 18.x? The script name says 17.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I branched the LLVM 17 branch here, so this release still uses LLVM 17 but has the LLVM patches that the new compiler needs here. The old and new branches are not compatible with the RISC-V compiler between OpenASIP revisions, so we cannot patch the new features to the old branch. Perhaps we can just keep this in main like it is and port the new LLVM release on top of this new branch (or a newer one) later?

@karihepola karihepola requested a review from pjaaskel November 12, 2024 16:56
@pjaaskel pjaaskel merged commit 943424a into main Nov 12, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants