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

Add macros for unaligned access #40

Merged
merged 1 commit into from
Jul 20, 2023

Conversation

vineetgarc
Copy link

No description provided.

Copy link

@nick-knight nick-knight left a comment

Choose a reason for hiding this comment

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

The RISC-V ISA spec (and profiles, etc.) appear to consistently use the word "misaligned". Is there a reason this proposal uses "unaligned"?

@vineetgarc
Copy link
Author

The RISC-V ISA spec (and profiles, etc.) appear to consistently use the word "misaligned". Is there a reason this proposal uses "unaligned"?

No real reason, just the terminology I'm more used to from past. I can certainly respin with new verbiage.

@vineetgarc vineetgarc force-pushed the topic-unaligned-access branch 2 times, most recently from f0cb8cc to 2b48b0f Compare May 11, 2023 17:04
riscv-c-api.md Outdated
Comment on lines 104 to 105
A typical complier could (but not necessarily) map fast variant to -mno-strict-align
and bad to -mstrict-align, if specified.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would like to drop that since clang has different behavior here, it just accept and ignore -mno-strict-align and -mstrict-align for RISC-V clang.

Or move it become a note to say GCC is using that way to decide which macro should be defined.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry not clear what you mean: clang ignores either of them ? and uses a different toggle to affect misaligned access codegen or doesn't support that at all (extremely unlikely)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry not clear what you mean: clang ignores either of them ? and uses a different toggle to affect misaligned access codegen or doesn't support that at all (extremely unlikely)

Yes, RISC-V clang accept that and then ignore that for now - clang did support misaligned code gen and some more are coming up[1], option handling in clang part can easily be implement/fix later.

Anyway just second thought about this: This should not major blocker issue here, just a minor implementation issue, and you already mention (but not necessarily).

@kito-cheng
Copy link
Collaborator

cc @asb @luismarques

Copy link
Collaborator

@kito-cheng kito-cheng left a comment

Choose a reason for hiding this comment

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

LGTM as a RISC-V GCC maintainer, but I would like to wait LLVM folks ack here.

@asb
Copy link

asb commented May 17, 2023

I think from #32, @aswaterman was concerned that _bad was unnecessarily perjorative and _avoid might be preferable.

I had a slight preference for zicclsm even if it's not very comprehensible just to avoid introducing too many ways of saying the same thing. But I don't feel strongly enough to block this. I think we should wait for @aswaterman's feedback though, as he participated in the other thread.

riscv-non-isa#32

Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
@vineetgarc
Copy link
Author

I think from #32, @aswaterman was concerned that _bad was unnecessarily perjorative and _avoid might be preferable.

Fair enough. Updated pull request with bad -> avoid

I had a slight preference for zicclsm even if it's not very comprehensible just to avoid introducing too many ways of saying the same thing.

I tend to disagree still, but we can agree to disagree :-)

But I don't feel strongly enough to block this. I think we should wait for @aswaterman's feedback though, as he participated in the other thread.

Sure thing.

Copy link
Collaborator

@kito-cheng kito-cheng left a comment

Choose a reason for hiding this comment

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

LGTM, and I am OK with those name

ping @aswaterman

@aswaterman
Copy link
Contributor

I'm OK with this scheme. I guess I agree with @asb in principle, but, like him, I don't think it's actually a problem, so I don't want to hold this up.

@asb
Copy link

asb commented Jul 20, 2023

Thanks Andrew. Sounds like we have consensus on this then? LGTM.

@kito-cheng
Copy link
Collaborator

Thanks @vineetgarc , gonna merge this :)

@kito-cheng kito-cheng merged commit a98e309 into riscv-non-isa:master Jul 20, 2023
@vineetgarc vineetgarc deleted the topic-unaligned-access branch July 21, 2023 17:43
yetingk added a commit to yetingk/llvm-project that referenced this pull request Sep 18, 2023
RISC-V C API introduce predefined macro to achieve hints about unaligned accesses [0].
This defines __riscv_misaligned_fast when using -mno-strict-align, otherwise,
defines __riscv_misaligned_avoid.

Note: This ignores __riscv_misaligned_slow which is also defined by spec.

The spec has mentioned riscv-non-isa/riscv-c-api-doc#40

[0]: riscv-non-isa/riscv-c-api-doc#40
yetingk added a commit to llvm/llvm-project that referenced this pull request Oct 26, 2023
…5756)

RISC-V C API introduced predefined macro to achieve hints about
unaligned accesses ([pr]). This patch defines __riscv_misaligned_fast
when using -mno-strict-align, otherwise, defines
__riscv_misaligned_avoid.

Note: This ignores __riscv_misaligned_slow which is also defined by
spec.

[pr]: riscv-non-isa/riscv-c-api-doc#40
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Oct 26, 2023
…vm#65756)

RISC-V C API introduced predefined macro to achieve hints about
unaligned accesses ([pr]). This patch defines __riscv_misaligned_fast
when using -mno-strict-align, otherwise, defines
__riscv_misaligned_avoid.

Note: This ignores __riscv_misaligned_slow which is also defined by
spec.

[pr]: riscv-non-isa/riscv-c-api-doc#40
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.

None yet

5 participants