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

JIT: explicitly handle __extendhfsf2 and friends #44975

Closed
wants to merge 1 commit into from
Closed

Conversation

vtjnash
Copy link
Sponsor Member

@vtjnash vtjnash commented Apr 13, 2022

A partial fix to #44829, but perhaps we will still have ABI problems with the sysimg (which may not link directly enough on linux in many cases)

@nalimilan @vchuravy I don't know how to make the linux linker work any better than this. The COFF and MACHO linkers would support two-level namespaces which would solve it, but the linux ELF implementations likely have problems with this still.

A partial fix to #44829, but perhaps we will still have ABI problems with the sysimg (which may not link directly enough on linux in many cases)

extern float __gnu_h2f_ieee(uint16_t param);
extern float __extendhfsf2(uint16_t param);
extern uint16_t __gnu_f2h_ieee(float param);
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

Suggested change
extern uint16_t __gnu_f2h_ieee(float param);
extern double __gnu_f2h_ieee(uint16_t param);

(not that it matters here, since we just need the declared prototype)

@vchuravy
Copy link
Sponsor Member

Another idea, adjust the TLI to have different names for these intrinsics? There are two reasons for trying to supply our own. 1. Some platforms were missing these, 2. We want our rounding semantics

@vtjnash
Copy link
Sponsor Member Author

vtjnash commented Apr 14, 2022

Ah, are we able to do that? Do you know how we can change these TLI names?

@vchuravy
Copy link
Sponsor Member

@vtjnash
Copy link
Sponsor Member Author

vtjnash commented Apr 15, 2022

I thought the TargetLibraryInfoImpl was private to the TargetMachine so that passes couldn't access or change it

@vchuravy
Copy link
Sponsor Member

That is true passes can't access it directly, but when you create your TM you can pass it a TLII and you can change it beforehand as Clang does in the link above

@alexfanqi
Copy link
Contributor

Could you also add __truncsfhf2, which is an equivalent version of __gnu_f2h_ieee. When working on the riscv port, I found llvm and gcc will emit a call to __truncsfhf2 on riscv instead of __gnu_f2h_ieee.

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

3 participants