Skip to content

[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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MouriNaruto
Copy link
Contributor

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

Copy link
Contributor

@arsenm arsenm left a 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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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

@MouriNaruto MouriNaruto force-pushed the extend-tli-checker-baseline branch from 7f4abf5 to ad97aad Compare November 1, 2024 16:55
@MouriNaruto
Copy link
Contributor Author

MouriNaruto commented Nov 1, 2024

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

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy paste comment

Copy link
Contributor Author

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

PS4 comment

Copy link
Contributor Author

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
Copy link
Contributor

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?

Copy link
Contributor Author

@MouriNaruto MouriNaruto Nov 1, 2024

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

@MouriNaruto MouriNaruto force-pushed the extend-tli-checker-baseline branch from ad97aad to 3da2f03 Compare November 1, 2024 17:34
@pogo59
Copy link
Collaborator

pogo59 commented Nov 1, 2024

As the author of ps4-tli-check.yaml: This test exists to test the tool rather than testing TLI itself. If you want to test TLI, then you should put tests wherever other TLI tests live, not in llvm/test/tools. (It's possible that there are no other TLI tests, in which case it is understandable that someone would be misled into thinking llvm/test/tools is where the TLI tests should go. But it's not.)

More specifically, ps4-tli-check.yaml tests whether the tool can handle different kinds of object files, and how it reacts to mismatches between TLI's settings and the content of an object file for some target (which just happens to be PS4, for no other reason than I was sure what functions were in the actual PS4 library). It is unfortunate that we have to pick some specific target for the test; that makes the test look like it is testing that the TLI for PS4 is correct. That is not the test's purpose; it is an artifact of the test implementation. We'd be better off if we had some kind of fake target that would never change, but we don't. I am open to suggestions for adding commentary to ps4-tli-check.yaml (and possibly renaming it) to make this clearer.

So, if you want to add tests that TLI is correct for a specific target then those tests should go somewhere else, probably under llvm/test/Analysis, and the test can use llvm-tli-checker to see whether some canned YAML list of functions matches what we expect to find. Those tests do not need to try different object-file types, they do not need to play games with the name of one of the functions (i.e. the ZDAPV variable), they should just run llvm-tli-checker once and expect the correct result.

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.

@MouriNaruto
Copy link
Contributor Author

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

@arsenm
Copy link
Contributor

arsenm commented Nov 5, 2024

So, if you want to add tests that TLI is correct for a specific target then those tests should go somewhere else, probably under llvm/test/Analysis, and the test can use llvm-tli-checker to see whether some canned YAML list of functions matches what we expect to find. Those tests do not need to try different object-file types, they do not need to play games with the name of one of the functions (i.e. the ZDAPV variable), they should just run llvm-tli-checker once and expect the correct result.

I'm confused then. What's the point of llvm-tli-checker if it's not already being used for this?

@pogo59
Copy link
Collaborator

pogo59 commented Nov 5, 2024

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?

@rengolin
Copy link
Member

rengolin commented Nov 6, 2024

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.

@arsenm
Copy link
Contributor

arsenm commented Jun 3, 2025

I added some tests running the output in #142536

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.

4 participants