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

feat(clang): add clang support #77

Merged
merged 11 commits into from
Sep 18, 2024
Merged

Conversation

ninolomata
Copy link
Member

No description provided.

@josecm
Copy link
Member

josecm commented Jul 20, 2023

I haven't delved too deeply into this yet, but I have a small suggestion. Could you please provide additional information in the PR description about how and in what targets you have tested this?

Also, for arm targets, we should test this with armclang, not just clang.

Plus, aarch32 must also be supported. @AfonsoSantos96 could you try to take a look at this?

@josecm josecm self-assigned this Jul 20, 2023
@danielRep
Copy link
Member

For Ubuntu 20.04, I've used

16:43 $ ~/toolchains/arm/clang+llvm-16.0.0-x86_64-linux-gnu-ubuntu-18.04/bin/clang-cpp --version
clang version 16.0.0
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /home/daniel/toolchains/arm/clang+llvm-16.0.0-x86_64-linux-gnu-ubuntu-18.04/bin

and I've effectively run bao on qemu-aarch64-virt platform:

=> go 0x50000000
## Starting application at 0x50000000 ...
Bao Hypervisor w/ clang compilation
Bao bare-metal test guest
cpu 1 up
cpu 2 up
cpu 3 up
cpu 0 up
cpu0: timer_handler
cpu1: ipi_handler
cpu2: ipi_handler
cpu3: ipi_handler
QEMU: Terminated

@danielRep
Copy link
Member

@ninolomata , could you separate the changes into x number of meaningful commits? It would be easier to understand some of the changes made. I feel that e.g. with the bao code changes it was an iterative process, maybe that would be sufficient to map it to significant commits.

@josecm
Copy link
Member

josecm commented Aug 15, 2023

Also, @ninolomata , could you clarify exactly what is need to compile the code with clang? I'm guessing:

make PLATFORM=... CONFIG=... LLVM=/path/to/llvm/install

Is my assumption correct?

I've installed clang via a package manager. So all the executables are in /usr/bin. Could you clarify how you have installed clang also?

@josecm
Copy link
Member

josecm commented Oct 20, 2023

Another point is that we should include build checks for clang now that we have a CI pipeline.

@DavidMCerdeira
Copy link
Member

I've rebase this branch with main, and replaced the proposed flag with CC_IS_* to distinguish between GCC and CLANG compilers both on the Makefile and in the source code (e.g., on src/arch/armv8/aarch64/boot.S).

I also propose to use the LLVM variable as a full path, so if one wants to use the package manager (to install clang llvm lld) it is possible to simply pass LLVM=/usr/bin/ as a Makefile argument.

Another change is replacing the use of CROSS_COMPILE variable to create the target architecture variable for clang, with ARCH_TARGET. I feel that CROSS_COMPILE variable having different meaning depending on the compiler used can be confusing.

@DavidMCerdeira DavidMCerdeira force-pushed the feat/clang branch 6 times, most recently from 444474f to b11b413 Compare July 25, 2024 10:51
@josecm
Copy link
Member

josecm commented Jul 26, 2024

@miguelafsilva5 I don't think those commit tags make sense. Maybe always use 'clang', e.g. fix(clang): fix inline functions

@josecm josecm changed the title feat(clang): add clang support for riscv64 and aarch64 feat(clang): add clang support Aug 19, 2024
@josecm josecm marked this pull request as ready for review August 22, 2024 09:49
@danielRep danielRep requested review from Diogo21Costa and removed request for miguelafsilva5 September 5, 2024 10:01
@danielRep danielRep assigned danielRep and unassigned josecm Sep 5, 2024
@Diogo21Costa
Copy link
Member

Tested on Ubuntu 22.04 with the following setup:

Ubuntu clang version 18.1.8 (++20240731024944+3b5b5c1ec4a3-1~exp1~20240731145000.144)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

Successfully ran the Bao bare-metal demo from the bao demos on qemu-aarch64-virt platform

Diogo21Costa
Diogo21Costa previously approved these changes Sep 6, 2024
@danielRep
Copy link
Member

Tested on Ubuntu 22.04 with the following setup:

Ubuntu clang version 18.1.8 (++20240731024944+3b5b5c1ec4a3-1~exp1~20240731145000.144)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

Successfully ran the Bao bare-metal demo from the bao demos on qemu-aarch64-virt platform

Double-checked. LGTM.

danielRep
danielRep previously approved these changes Sep 18, 2024
ninolomata and others added 11 commits September 18, 2024 14:39
Signed-off-by: Bruno Sa <bruno.vilaca.sa@gmail.com>
Signed-off-by: David Cerdeira <davidmcerdeira@gmail.com>
Clang assembler requires valid assembly; otherwise, an error is thrown.
Instead of using "->" as the token to parse for assembly macro defines,
we define the tokens "#", which is used for comments in assembly files.
The generated output will be valid for both clang and gcc.

Signed-off-by: Bruno Sa <bruno.vilaca.sa@gmail.com>
This commit fixes relocation errors identified by the clang compiler and
marks the .gtl_page_tables section as no data otherwise there is a type
mismatch.

Signed-off-by: Bruno Sa <bruno.vilaca.sa@gmail.com>
This commit fixes some issues when using clang:
- remove .directive .func which is not recognized by clang assembler;
- make gtlb_page_tables as no data to avoid section type mismatch;
- remove general-regs-only in aarch32 this is not recognized by the
  clang assembler
- remove mov instructions with flexible second operand since they are
  not recognized by the clang assembler;

Signed-off-by: Bruno Sa <bruno.vilaca.sa@gmail.com>
Signed-off-by: David Cerdeira <davidmcerdeira@gmail.com>
These flags are enabled in Clang compiler by default.
Adding these flags minimized the discrepancies between GCC and Clang

Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
Clang compiler only accepts fallthrough annotation to be done using
attribute(fallthrough). This is can be enforced in gcc using the flag
-Wimplicit-fallthrough=5

Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
Both MISRA and clang compiler force inline functions to be static.
Functions that are used in multiple sources stop being inline because they
cannot be static.
The other inline functions become static and their prototype is removed from
the header

Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
The way this size was being calculated using the linker generated a
linking error when targetting risc-v.

Signed-off-by: Jose Martins <josemartins90@gmail.com>
Signed-off-by: Jose Martins <josemartins90@gmail.com>
Signed-off-by: Jose Martins <josemartins90@gmail.com>
@danielRep danielRep merged commit 02790d7 into bao-project:main Sep 18, 2024
27 checks 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.

7 participants