Fix TCC compiler integration for parallel compilation#27033
Fix TCC compiler integration for parallel compilation#27033medvednikov merged 2 commits intomasterfrom
Conversation
Three independent issues caused parallel_cc_failure_test.v and
compiler_errors_test.v (missing_shader_header_1.vv) to fail under
`VFLAGS=-cc tcc -no-retry-compilation`:
1. parallel_cc passed `-B${vroot}/thirdparty/tcc` so tcc could find
libtcc1.a, but `-B${path}` makes tcc look for system headers at
`${path}/include` (which only ships libtcc.h), so `<stddef.h>` lookups
failed. Drop the flag and instead chdir to vroot before invoking tcc,
matching what builder/cc.v already does for thirdparty objects.
2. The non-Windows fallback branch of `VV_LOC` was always `static`, even
in `-parallel-cc` mode. With tcc that branch is reached on Linux and
the resulting per-translation-unit static functions could not satisfy
cross-file references at link time. Mirror the Windows branch and
make `VV_LOC` empty when `_VPARALLELCC` is defined.
3. `get_guarded_include_text` and `get_inttypes_or_stdint_include_text`
bypassed `__has_include` for `__TINYC__`, so missing shader headers
under tcc emitted tcc's terse `'foo.h' not found` instead of the
`VERROR_MESSAGE` text the checker tests assert on. tcc 0.9.28 supports
`__has_include` correctly, so use the same path as other compilers.
Also detect tcc's "defined twice" warning (which it prints to stderr but
still exits 0 from when an executable is produced) so that
test_parallel_cc_fails_when_final_linking_fails sees a proper failure.
https://claude.ai/code/session_012PDdNjq22fSCxGV3PiTBMT
|
I thought we were going to drop tcc once v2 was done. There would no longer be a need for it. Even bootstrapping a fresh install could be accomplished other ways. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5b9fe0e98
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| prev_cwd := os.getwd() | ||
| if b.ccoptions.cc == .tcc { | ||
| os.chdir(b.pref.vroot) or {} | ||
| } |
There was a problem hiding this comment.
Switching the process working directory to b.pref.vroot here breaks builds that rely on relative tool/flag paths in -parallel-cc mode with TCC. parallel_cc_compiler_path() uses b.pref.ccompiler verbatim, and compile/link args also carry user/env flags as provided, so values like -cc ./tcc, CFLAGS=-I./include, or LDFLAGS=-L./lib are now resolved relative to the V root instead of the project directory and can fail to find the compiler, headers, or libraries.
Useful? React with 👍 / 👎.
|
This is a V1 fix, not V2. Have to make all tests pass. |
The previous chdir-to-vroot approach risked breaking user-supplied
relative paths (e.g. `-cc ./tcc`, `CFLAGS=-I./include`,
`LDFLAGS=-L./lib`), since they would resolve relative to the V root
instead of the project directory.
Pass absolute `-B"${vroot}/thirdparty/tcc/lib/tcc"` for system header
discovery, plus `-L"${vroot}/thirdparty/tcc/lib/tcc"` so the linker
finds libtcc1.a / bt-*.o, and let the cwd remain whatever v was
invoked from. User-supplied relative flags continue to resolve
against the project directory as before.
https://claude.ai/code/session_012PDdNjq22fSCxGV3PiTBMT
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf0eeefa9a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| |#if defined(__has_include) | ||
| |#if __has_include(${iname}) |
There was a problem hiding this comment.
Removing the defined(__TINYC__) fast path makes get_guarded_include_text() rely on __has_include for TinyCC whenever the prelude path does not inject the #undef __has_include guard (for example -no-preludes or -custom-prelude). In those modes, a #include hash stmt now goes through __has_include and can incorrectly hit the generated #error on TCC, while the previous code always emitted a direct include for TinyCC. This is a regression for TCC builds that customize or disable preludes.
Useful? React with 👍 / 👎.
This will consume a lot of time, and it would be better if v2 could pass all tests as soon as possible if possible. |
|
All the tests would have to be converted over, all the CI jobs would have to be updated, etc. If the goal is to pass all tests with v1, then switch to v2, that's alright. |
4 participants
Summary
This PR fixes several issues with TCC (Tiny C Compiler) support, particularly in the context of parallel C compilation. The changes address path resolution problems, duplicate symbol detection, and header inclusion guards.
Key Changes
Parallel CC working directory handling: Changed TCC compilation to execute from the V root directory, since TCC resolves include and library search paths relative to the invocation directory. This ensures
thirdparty/tcc/lib/tcc/...lookups work correctly for standard headers and runtime objects.TCC duplicate symbol detection: Added detection for TCC's specific behavior where duplicate symbol errors are reported via stderr but the linker still exits with code 0. The build now properly fails when
defined twiceappears in TCC's output.Simplified header inclusion guards: Removed TCC-specific
__TINYC__preprocessor checks fromget_guarded_include_text()andget_inttypes_or_stdint_include_text()functions, relying instead on the more portable__has_includefeature that TCC supports.Parallel compilation symbol visibility: Modified the
VV_LOCmacro definition to be empty (instead ofstatic) when_VPARALLELCCis defined, allowing proper symbol visibility across separately compiled object files in parallel compilation mode.Implementation Details
os.getwd()andos.chdir()with defer to safely manage working directory changes during TCC compilationb.ccoptions.cc == .tcc_VPARALLELCCflag controls whether symbols should be static or globally visible across compilation unitshttps://claude.ai/code/session_012PDdNjq22fSCxGV3PiTBMT