Repository navigation
Build the C builtins without C library headers - #3977
Conversation
1a65d4b to
0d93d36
Compare
382b2d5 to
86c4ff4
Compare
aneshlya
left a comment
There was a problem hiding this comment.
Reviewed at 86c4ff4fb988f27f016e96ff987f417b6f29897d.
The overall approach looks sound and is a maintainability improvement: the small, stable libc interface is easier to maintain than the previous host/target header-selection matrix. Emitting bitcode directly also fixes the hidden compiler-error problem.
I would address the PlayStation ABI selection before merging. In get_target_flags(), the PlayStation triple is ${arch}-scei-ps. LLVM does not recognize ps as a PlayStation OS, so the new declarations fall through to the glibc branch and generate sysconf(84).
I compiled this PR's builtin source with LLVM 20, 21, 22, and 23 and consistently obtained:
| Triple | stdout symbol | sysconf argument |
|---|---|---|
x86_64-scei-ps (current) |
stdout |
84 |
x86_64-scei-ps4 |
__stdoutp |
58 |
x86_64-sie-ps5 |
__stdoutp |
58 |
Public PS4 ABI definitions identify 84 as _SC_THREAD_CPUTIME, while 58 is _SC_NPROCESSORS_ONLN. The current code therefore selects CPU-time support instead of the processor count. ISPC also aliases the PS4 builtin library for PS5.
The smallest correction is set(triple ${arch}-scei-ps4), with a lit regression test for both --target-os=ps4 and ps5 checking sysconf(58). The malformed triple predates this PR, but this seems an appropriate place to fix it because the change explicitly selects the target libc ABI.
The stdout spelling alone is not evidence of a link failure: PS4 export records contain both stdout and __stdoutp. I have not validated against Sony's SDK or hardware; the finding combines reproduced compiler output with public ABI evidence.
Additional maintainability suggestions:
- Replace the unconditional glibc fallback with an explicit supported Linux branch and an error for unsupported targets. This would catch malformed triples instead of silently assigning them an ABI.
- Record the ABI sources and minimum versions beside the declarations/constants, especially Android API 23.
- Add a build-level negative test for compiler-error propagation. The new lit tests validate successful builtin generation, but would still pass if the old pipeline were restored while compilation succeeds.
- Finish removing the obsolete header-package dependency from Windows CI: install-build-deps.ps1 still downloads and extracts
libgw32c-0.4-lib.zip, and fails the job if downloading fails. The separategnuwin32-m4tool is still needed.
Validation: built the PR with LLVM 23 and cross-target, macOS, ARM32, RISC-V, and PowerPC support; checked all 17 generated Unix builtin variants; ran the lit suite (706 passed, 6 expected failures, 105 unsupported, no unexpected failures); and ran the new runtime test on Linux x86 and x86-64. Nine CMake configuration cases behaved as documented. An injected compiler error reproduced the original bug: the base build succeeds and creates empty bitcode, while this PR fails and creates none.
86c4ff4 to
62275de
Compare
dbabokin
left a comment
There was a problem hiding this comment.
LGTM.
The reason we didn't do that earlier is that we were not comfortable with relying on the specific constants and stability of these values. But the tests and safeguards that this PR introduces look good. I think the reduction in the complexity of cross-builds outweighs the risks.
|
Thanks, this looks good. There are several little things worth cleaning up before merging: 1. The m4 builtins still pipe into llvm-as ( COMMAND ${M4_EXECUTABLE} -I${include} -DBUILD_OS=${OS_UP} -DRUNTIME=${bit} ${input}
| \"${LLVM_AS_EXECUTABLE}\" -o ${bc}This is the same issue the PR fixes for the C builtins. The pipe drops m4's exit status, and 2. The PR adds Nits
|
Declare the small Unix libc interface directly and select the target stdout symbol and sysconf constant. Preserve fputs and stdout-only flushing. Emit CPU and WASM bitcode directly so compiler errors stop the build instead of producing incomplete libraries. Remove obsolete target-header dependencies from CMake, CI and superbuild while retaining Windows SDK requirements and the existing Termux and Windows-cross target selections. Add libc, architecture, processor-count and stdout-flushing regression coverage for #3887. Correct the PlayStation builtin triple to match ISPC target selection, and fix the iOS lit feature and its ARM backend prerequisite.
Record ABI references and reject unsupported target OSes. Exercise production builtin generation with an intentional compiler error from check-all, keeping check-one focused. Call the Windows emcc batch launcher so subsequent builtin commands execute after direct bitcode generation.
2a16d7a to
65f8a6c
Compare
Description
The C builtins for
print()andnum_cores()previously depended on target libc headers. Missing or incompatible headers could cause compilation errors hidden by theclang | llvm-aspipeline, producing empty bitcode and leaving these functions unresolved at link time.This PR declares the small Unix libc interface directly and selects the target's
stdoutsymbol and_SC_NPROCESSORS_ONLNconstant.CPU and WASM builtins now emit bitcode directly. Compilation errors stop the build instead of silently producing incomplete libraries; targets with failed builtins are not automatically disabled. On Windows, the WASM build command uses
callto return from theemccbatch launcher.Unix builtin generation no longer requires target libc headers, removing the Android NDK and obsolete GNUWIN32 libc-header dependencies. Windows builds still require
m4, and Windows cross-target builds still require Windows SDK and Visual C++ headers. Without those SDK paths, the Windows target is disabled on Linux and macOS. Target SDKs and libraries may still be required when linking generated programs.macOS ARM target support no longer depends on the detected macOS SDK version.
The PlayStation builtin triple is corrected to
x86_64-scei-ps4, with PS4 and PS5 regression checks forsysconf(58). Unsupported target libc configurations now produce an explicit compilation error instead of falling through to the glibc declarations. Comments document the ABI sources and Android API 23 requirement.Related Issue
Checklist
clang-format.