Allow assembling with LLVM #391
Conversation
Thanks for the review - what should I update the date and version of changelog entry to to allow merging this? |
|
Todays date, and bump the version by the minor digit.
…On Wed, Sep 27, 2023 at 7:56 AM Alexander Richardson < ***@***.***> wrote:
Looks good to me
Thanks for the review - what should I update the date and version of
changelog entry to to allow merging this?
—
Reply to this email directly, view it on GitHub
<#391 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHPXVJRZFTUJGLMWQKPMA6TX4Q5ADANCNFSM6AAAAAA5H6OVQA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
When assembling with LLVM, this macro produces the following: `error: directional label undefined`. This happens because it compares the symbol value rather than performing a textual comparison. At the point the 3f label is referenced it is actually a backwards reference so we would have to use 3b instead. Fix this by using a `.ifc` string comparison directive instead.
The LLVM assembler does not accept string comparisons such as `\__MODE__\() == M` and instead requires the .ifc/.ifnc directive. This requires expanding some .elseif to nested else since GNU AS does not allow use of `.elseifc` (although in this case LLVM does). Another options would have been `.set M, 0x<constant>` to evaluate the expression as integers but I feel that using an explicit string comparison is cleaner. With LLVM patched to include llvm/llvm-project#67377 I was able to use riscof to compare RV64 sail assembled with LLVM vs. RV64 sail assembled with GCC.
arichardson
force-pushed
the
assemble-with-llvm
branch
from
3f8a930
to
30b7f34
Compare
Done, thanks |
|
It turns out that this breaks the GCC. You tested it - but those test also trapped to Mmode, so it didn't make a difference. |
| .endm | ||
|
|
||
| .macro XCSR_RENAME __MODE__ // enable CSR names to be parameterized, V,S merged | ||
| .ifc \__MODE__\, S |
There was a problem hiding this comment.
I see a stray \ here maybe that causes the issue?
There was a problem hiding this comment.
Umer thinks that doesn't matter.
I changed the RENAME code so there are separate RENAME_M, _S, V macros, and the the RENAME[V]CSR macros just call those - but in unnested if/endif pairs.
If your code works with GCC, then these won't even compile, since none of the .ifs will match, and you'll get an undefined error when you try to compile it.
If you can test this with GCC , then make the above changes (and optionally #define RVTEST_STRAP_ROUTINE).
IF that compiles, then your fix works, else it will cause a compile failure.
There was a problem hiding this comment.
Also: pull from the latest first....please.
|
It is working now |
Description
This PR adjusts the tests frameworks such that it can be assembled with an upcoming version of LLVM.
\__MODE__\() == Min .if and instead requires the .ifc/.ifnc directive.With LLVM patched to include llvm/llvm-project#67377
I was able to use riscof to compare RV64 sail assembled with LLVM vs.
RV64 sail assembled with GCC.
See individual commit messages for more details (not sure what the workflow is but since there are detailed commit messages please don't squash and merge).
Related Issues
NA
Ratified/Unratified Extensions
List Extensions
NA
Reference Model Used
Mandatory Checklist:
Since this does not add new tests, some of these are N/A?
Optional Checklist: