-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[TLI Checker] Extend the targets for Linux, macOS and Windows. #114556
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe there are any architecture deviations in the call availability in these cases. You can merge the 32/64-bit cases into multiple run lines in one file
The tests are also failing the bot
@@ -0,0 +1,1168 @@ | |||
# REQUIRES: aarch64-registered-target |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not require the target
@@ -0,0 +1,1168 @@ | |||
# REQUIRES: x86-registered-target |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does not require the target
@@ -0,0 +1,1168 @@ | |||
# REQUIRES: x86-registered-target |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does not require the target
7f4abf5
to
ad97aad
Compare
I have merged the 32/64-bit cases for each platform and removed the "# REQUIRES: *-registered-target" line. Kenji Mouri |
Type: STT_FUNC | ||
Section: .text | ||
Binding: STB_GLOBAL | ||
# The rest of these are the remaining symbols needed for PS4. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copy paste comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will fix that.
Type: SHT_PROGBITS | ||
DynamicSymbols: | ||
# This is an undefined symbol that is known to TLI but not in the | ||
# available set for PS4, showing the tool will ignore undefined symbols. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PS4 comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will fix that.
## This is a large file so it's worth telling lit to stop here. | ||
# END. | ||
|
||
--- !ELF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What exactly does the yaml input here represent? Is this input supposed to be a list of every possible libcall? Should there be one common data input for all the tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What exactly does the yaml input here represent?
I just copied the content from ps4-tli-check.yaml.
Is this input supposed to be a list of every possible libcall? Should there be one common data input for all the tests?
I have no idea but I know there seems only a little difference at least for Windows UCRT export symbols.
Here are the x86-32 UCRT export symbols, the 10.0.10240 UCRT export symbols for all targets is excluded:
__control87_2
__CxxLongjmpUnwind@4
__intrinsic_abnormal_termination
__intrinsic_setjmp
__libm_sse2_acos
__libm_sse2_acosf
__libm_sse2_asin
__libm_sse2_asinf
__libm_sse2_atan
__libm_sse2_atan2
__libm_sse2_atanf
__libm_sse2_cos
__libm_sse2_cosf
__libm_sse2_exp
__libm_sse2_expf
__libm_sse2_log
__libm_sse2_log10
__libm_sse2_log10f
__libm_sse2_logf
__libm_sse2_pow
__libm_sse2_powf
__libm_sse2_sin
__libm_sse2_sinf
__libm_sse2_tan
__libm_sse2_tanf
_chkesp
_CIacos
_CIasin
_CIatan
_CIatan2
_CIcos
_CIcosh
_CIexp
_CIfmod
_CIlog
_CIlog10
_CIpow
_CIsin
_CIsinh
_CIsqrt
_CItan
_CItanh
_crt_debugger_hook
_CxxThrowException@8
_EH_prolog
_except_handler2
_except_handler3
_except_handler4_common
_ftol
_global_unwind2
_libm_sse2_acos_precise
_libm_sse2_asin_precise
_libm_sse2_atan_precise
_libm_sse2_cos_precise
_libm_sse2_exp_precise
_libm_sse2_log_precise
_libm_sse2_log10_precise
_libm_sse2_pow_precise
_libm_sse2_sin_precise
_libm_sse2_sqrt_precise
_libm_sse2_tan_precise
_local_unwind2
_local_unwind4
_longjmpex
_NLG_Dispatch2
_NLG_Return
_NLG_Return2
_seh_longjmp_unwind@4
_seh_longjmp_unwind4@4
_set_SSE2_enable
_setjmp3
_statusfp2
Here are the x86-64 10.0.19041 UCRT export symbols, the 10.0.10240 UCRT export symbols for all targets is excluded:
__CxxFrameHandler4
__std_terminate
ad97aad
to
3da2f03
Compare
As the author of More specifically, So, if you want to add tests that TLI is correct for a specific target then those tests should go somewhere else, probably under I apologize if some of this comment sounds a little upset. This exact point has come up a few times, which is frustrating, and we probably should try to keep the same misunderstanding from happening again. |
Thanks. Got it. However, I'm sorry that I need some time to learn more about the LLVM codebase and test locally to achieve this goal. Kenji Mouri |
I'm confused then. What's the point of llvm-tli-checker if it's not already being used for this? |
I'll try again. At the very least, Sony is using llvm-tli-checker downstream, running it against the actual libraries they deliver. (I know this because I wrote those downstream tests.) That's what the tool was originally intended for. I suspect that Sony is not the only vendor using it this way, but I am not certain (@rengolin was a significant reviewer of the original tool, so perhaps ARM or Linaro are using it as well). My impression is that now there is a desire to create LLVM tests that ensure TLI has selected the correct set of available functions for a particular target. There are no such existing tests of TLI. llvm-tli-checker was designed to check TLI against vendor-provided libraries, but you can fake one up with yaml easily enough. Note that you would be doing that in order to check the correctness of TLI, not llvm-tli-checker, so such a test would belong with other tests of TLI or the library TLI belongs to (i.e., Analysis). ps4-tli-check.yaml is coincidentally comparing the list of available functions against a canned set for the PS4 target, but that's not the intent of the test. That test is making sure the tool operates correctly. It's somewhat analogous to llvm-dwarfdump. We have tests of llvm-dwarfdump itself, to make sure the tool does something reasonable when given certain canned inputs; but then we also use llvm-dwarfdump as a test tool against "live" objects to make sure the compiler is producing DWARF that we expect. In the case of llvm-tli-checker, we have tests making sure the tool does something reasonable when given certain canned inputs; but there are NO uses of llvm-tli-checker within upstream LLVM that use it as a test tool in the same way we use llvm-dwarfdump. llvm-tli-checker is used that way only downstream (currently). Does that help? |
You have no idea how many frames I had to unwind through to remember what you're talking about! 😅 Would it be possible to add some simple tests in that direction upstream? Even if just basic ones, so that people don't get confused in the future? Not necessarily for this PR of course, but some dynamic checks would be nice. Question is, would we need some weird object files or piping tools through each other? I'm guessing this is part of the reason they don't exist. |
I added some tests running the output in #142536 |
According to the suggestions (#114536 (comment)) from @arsenm, I try to create new baseline TLI checker tests PR first.
Based on the "Tier 1 with Host Tools" section in https://doc.rust-lang.org/nightly/rustc/platform-support.html, I have added some typical targets for TLI checker tests.
Kenji Mouri