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 demangling for f16 and f128 #64

Closed
wants to merge 1 commit into from
Closed

Conversation

tgross35
Copy link

@tgross35 tgross35 commented Aug 31, 2023

k was selected for f16/half and q for f128/quad.

These are different from LLVM's defaults which are Dh for half (link) and g for quad (link). I don't know if there is any reason to mirror LLVM here because it would mean adding multi character tags for primitives. I just used q because it it's more obviously in line with f for float/f32 and d for double/f64.

Part of rust-lang/rust#114607

`k` was selected for `f16` and `q` for `f128`. These are different from LLVM's
defaults.
@tgross35 tgross35 marked this pull request as ready for review December 4, 2023 03:43
@tgross35
Copy link
Author

tgross35 commented Dec 4, 2023

The f16 & f128 RFC was merged so I think it is OK to decide on mangling. Marking it as ready for review @Mark-Simulacrum

@tgross35
Copy link
Author

tgross35 commented Dec 4, 2023

I think we probably don't need different mangling for PowerPC that LLVM seems to have but could use confirmation https://github.com/llvm/llvm-project/blob/fdef7952cbc26fd31d0d92a4f14dde58bc461fe9/clang/lib/Basic/Targets/PPC.h#L351 patch llvm/llvm-project@0461534

@tgross35
Copy link
Author

tgross35 commented Mar 6, 2024

@rcvalle / @michaelwoerister, we need this mangling at some point - any guidance on how to proceed?

Itanium uses Dh for f16 but I think that may conflict with some dyn signature.

@michaelwoerister
Copy link
Member

We'll need to take a look at what the grammar allows. Is this implemented in the compiler already? Will these types be primitive types in Rust?

@michaelwoerister
Copy link
Member

I just saw the links in the PR message. Will read those 🙂

@tgross35
Copy link
Author

tgross35 commented Mar 6, 2024

We'll need to take a look at what the grammar allows. Is this implemented in the compiler already? Will these types be primitive types in Rust?

The primitives exist in the compiler but not yet fully exposed (rust-lang/rust#121926 coming soon). Currently it does the itanium mangling and just ICEs with v0 mangling.

@michaelwoerister
Copy link
Member

michaelwoerister commented Mar 6, 2024

k and q seem like good picks to me.

@ehuss suggested here that we extend the mangling scheme by updating the official docs. I think that is a good idea. Would you mind opening a PR that adds the two cases to the basic types section and then we can do a compiler team FCP to sign off on the change.

@rcvalle
Copy link
Member

rcvalle commented Mar 6, 2024

I added a comment at rust-lang/rust#121728 (comment), but this is for CFI encoding and cross-language CFI compatibility and independent/dissociated to Rust mangling/demangling and a different encoding can be used for it.

@tgross35
Copy link
Author

tgross35 commented Mar 6, 2024

Thanks for the pointers, I opened rust-lang/rust#122106 to follow up

@michaelwoerister
Copy link
Member

I think this can be closed now, right @tgross35?

@tgross35
Copy link
Author

Totally forgot abut this, thanks!

@tgross35 tgross35 closed this Jun 20, 2024
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.

3 participants