Fix val<bool>(TypedValueRef&) to propagate actual values#289
Draft
PhilippGrulich wants to merge 6 commits into
Draft
Fix val<bool>(TypedValueRef&) to propagate actual values#289PhilippGrulich wants to merge 6 commits into
PhilippGrulich wants to merge 6 commits into
Conversation
The single-arg constructor hardcoded value=false for all traced boolean results, causing incorrect runtime values in val<bool> instances produced by logical ops (AND, OR, NOT), comparisons (==, !=, <, >, <=, >=), and pointer comparisons. This could lead to miscompilation when the concrete value matters during tracing, e.g. when an overloaded && embeds it as a constant in the execution trace. Add a two-arg constructor val(TypedValueRef&, bool) and use it at all 16 call sites where the actual result is known: lOr, lAnd, lNot, eq, neq, pointer comparisons, enum comparisons, arithmetic comparisons (via macro), and bool& load operations. https://claude.ai/code/session_01WRNQDzY1277GnG5knAp6hM
Pointer values from traced arithmetic (e.g. mask[i]) are nullptr during tracing because base_ptr_val(TypedValueRef) initializes value to nullptr. The previous commit moved the dereference before the tracing guard, causing a segfault in demos like conditionalSum. Revert the load operator to dereference only in the non-tracing path. https://claude.ai/code/session_01WRNQDzY1277GnG5knAp6hM
These .host.cpp files are expected-output reference data compared against generated code in GPU tests — reformatting them breaks the comparison. https://claude.ai/code/session_01WRNQDzY1277GnG5knAp6hM
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Tracing Benchmark
Details
| Benchmark suite | Current: 6e60556 | Previous: f9cd28f | Ratio |
|---|---|---|---|
ir_add |
774.845 ns (± 65.8493) |
757.379 ns (± 38.5584) |
1.02 |
ir_ifThenElse |
1.56668 us (± 179.929) |
1.58461 us (± 136.165) |
0.99 |
ir_deeplyNestedIfElse |
3.25964 us (± 296.107) |
3.40826 us (± 363.019) |
0.96 |
ir_loop |
1.61331 us (± 155.644) |
1.63694 us (± 149.086) |
0.99 |
ir_ifInsideLoop |
2.81992 us (± 327.109) |
2.83439 us (± 293.378) |
0.99 |
ir_loopDirectCall |
1.82718 us (± 204.679) |
1.80584 us (± 177.074) |
1.01 |
ir_pointerLoop |
1.9569 us (± 206.287) |
1.97161 us (± 153.779) |
0.99 |
ir_staticLoop |
1.46098 us (± 148.18) |
1.44956 us (± 119.501) |
1.01 |
ir_fibonacci |
1.7484 us (± 261.896) |
1.72022 us (± 143.401) |
1.02 |
ir_gcd |
1.468 us (± 110.909) |
1.4549 us (± 101.976) |
1.01 |
ir_nestedIf10 |
7.31369 us (± 590.35) |
7.70943 us (± 540.522) |
0.95 |
ir_nestedIf100 |
92.2256 us (± 5.52465) |
93.6887 us (± 16.2569) |
0.98 |
ir_chainedIf10 |
11.4715 us (± 1.38389) |
11.594 us (± 826.837) |
0.99 |
ir_chainedIf100 |
161.162 us (± 8.64854) |
169.138 us (± 8.3263) |
0.95 |
exec_bc_addOne |
39.1 ns (± 10.6116) |
36.0837 ns (± 6.3975) |
1.08 |
exec_mlir_addOne |
281.583 ns (± 8.53594) |
258.154 ns (± 2.33301) |
1.09 |
exec_cpp_addOne |
4.06357 ns (± 0.789533) |
3.54853 ns (± 0.249363) |
1.15 |
exec_interpreted_addOne |
40.8637 ns (± 2.10564) |
37.1985 ns (± 2.33453) |
1.10 |
trace_add |
2.39186 us (± 212.296) |
2.27731 us (± 155.69) |
1.05 |
completing_trace_add |
2.46165 us (± 292.083) |
2.32405 us (± 143.855) |
1.06 |
trace_ifThenElse |
8.87676 us (± 979.773) |
8.63915 us (± 776.354) |
1.03 |
completing_trace_ifThenElse |
4.7465 us (± 495.371) |
4.54071 us (± 401.003) |
1.05 |
trace_deeplyNestedIfElse |
26.7625 us (± 3.92907) |
25.5445 us (± 1.57172) |
1.05 |
completing_trace_deeplyNestedIfElse |
14.4297 us (± 3.59707) |
12.559 us (± 987.646) |
1.15 |
trace_loop |
8.67687 us (± 802.101) |
8.80429 us (± 1.55155) |
0.99 |
completing_trace_loop |
4.71894 us (± 528.767) |
4.79104 us (± 376.911) |
0.98 |
trace_ifInsideLoop |
17.3411 us (± 1.9834) |
16.6688 us (± 1.56827) |
1.04 |
completing_trace_ifInsideLoop |
8.85044 us (± 1.28589) |
8.40114 us (± 807.253) |
1.05 |
trace_loopDirectCall |
9.01899 us (± 1.34381) |
8.6864 us (± 697.439) |
1.04 |
completing_trace_loopDirectCall |
5.00475 us (± 738.075) |
4.86476 us (± 354.791) |
1.03 |
trace_pointerLoop |
15.0882 us (± 2.43183) |
14.0902 us (± 1.18648) |
1.07 |
completing_trace_pointerLoop |
10.8776 us (± 1.41125) |
10.1206 us (± 990.611) |
1.07 |
trace_staticLoop |
7.5272 us (± 826.947) |
7.21602 us (± 496.139) |
1.04 |
completing_trace_staticLoop |
7.94469 us (± 1.12644) |
7.43532 us (± 991.516) |
1.07 |
trace_fibonacci |
10.8431 us (± 1.91139) |
10.0814 us (± 861.346) |
1.08 |
completing_trace_fibonacci |
6.73053 us (± 981.774) |
6.00031 us (± 426.095) |
1.12 |
trace_gcd |
8.06627 us (± 1.11425) |
7.79585 us (± 648.305) |
1.03 |
completing_trace_gcd |
4.22273 us (± 462.694) |
3.87126 us (± 272.452) |
1.09 |
trace_nestedIf10 |
42.3538 us (± 7.69213) |
38.0283 us (± 3.64798) |
1.11 |
completing_trace_nestedIf10 |
41.5068 us (± 8.52588) |
37.3711 us (± 3.21573) |
1.11 |
trace_nestedIf100 |
1.37577 ms (± 40.7068) |
1.35476 ms (± 22.6997) |
1.02 |
completing_trace_nestedIf100 |
1.40129 ms (± 43.0203) |
1.35268 ms (± 26.016) |
1.04 |
trace_chainedIf10 |
100.167 us (± 6.83129) |
97.7583 us (± 4.73759) |
1.02 |
completing_trace_chainedIf10 |
52.8245 us (± 9.00842) |
48.3258 us (± 3.59147) |
1.09 |
trace_chainedIf100 |
4.4437 ms (± 45.3854) |
4.43042 ms (± 29.6138) |
1.00 |
completing_trace_chainedIf100 |
2.35716 ms (± 62.787) |
2.24075 ms (± 31.9951) |
1.05 |
exec_mlir_add |
10.9396 ns (± 1.44696) |
10.6651 ns (± 0.946586) |
1.03 |
exec_mlir_fibonacci |
13.3781 us (± 1.63988) |
13.4703 us (± 2.01788) |
0.99 |
exec_mlir_sum |
523.725 us (± 19.007) |
527.422 us (± 21.2529) |
0.99 |
exec_cpp_add |
4.65856 ns (± 0.564179) |
4.69868 ns (± 0.745035) |
0.99 |
exec_cpp_fibonacci |
96.8271 us (± 9.95927) |
96.6361 us (± 9.26507) |
1.00 |
exec_cpp_sum |
35.9254 ms (± 111.467) |
35.9426 ms (± 115.287) |
1.00 |
exec_bc_add |
46.14 ns (± 9.85651) |
44.8635 ns (± 8.015) |
1.03 |
exec_bc_fibonacci |
841.096 us (± 17.7677) |
859.028 us (± 13.0138) |
0.98 |
exec_bc_sum |
179.955 ms (± 1.08084) |
186.067 ms (± 1.36597) |
0.97 |
exec_asmjit_add |
3.59946 ns (± 0.536413) |
3.48273 ns (± 0.424618) |
1.03 |
exec_asmjit_fibonacci |
21.2775 us (± 3.79629) |
20.8625 us (± 3.3694) |
1.02 |
exec_asmjit_sum |
4.85306 ms (± 68.7941) |
4.87965 ms (± 312.598) |
0.99 |
exec_bc_add_noRegAlloc |
45.6664 ns (± 7.65811) |
44.3118 ns (± 3.89068) |
1.03 |
exec_bc_add_regAlloc |
44.6369 ns (± 6.11576) |
44.599 ns (± 5.15383) |
1.00 |
exec_bc_fibonacci_noRegAlloc |
842.247 us (± 20.3794) |
852.767 us (± 11.3762) |
0.99 |
exec_bc_fibonacci_regAlloc |
843.509 us (± 14.3913) |
875.208 us (± 25.409) |
0.96 |
exec_bc_sum_noRegAlloc |
178.978 ms (± 301.011) |
185.974 ms (± 222.438) |
0.96 |
exec_bc_sum_regAlloc |
179.648 ms (± 1.18245) |
185.982 ms (± 165.251) |
0.97 |
comp_mlir_add |
5.4993 ms (± 138.918) |
5.55479 ms (± 56.4997) |
0.99 |
comp_mlir_ifThenElse |
6.26061 ms (± 198.793) |
6.14317 ms (± 38.4483) |
1.02 |
comp_mlir_deeplyNestedIfElse |
4.99868 ms (± 137.871) |
5.06889 ms (± 99.0918) |
0.99 |
comp_mlir_loop |
7.19682 ms (± 234.616) |
7.14743 ms (± 31.0585) |
1.01 |
comp_mlir_ifInsideLoop |
28.6272 ms (± 472.863) |
28.6125 ms (± 100.629) |
1.00 |
comp_mlir_loopDirectCall |
11.7752 ms (± 295.524) |
11.792 ms (± 44.0495) |
1.00 |
comp_mlir_pointerLoop |
27.6466 ms (± 329.088) |
27.7179 ms (± 76.6326) |
1.00 |
comp_mlir_staticLoop |
4.98521 ms (± 130.838) |
5.03112 ms (± 38.6111) |
0.99 |
comp_mlir_fibonacci |
10.457 ms (± 352.551) |
10.4618 ms (± 76.5332) |
1.00 |
comp_mlir_gcd |
9.34488 ms (± 170.175) |
9.43388 ms (± 42.8866) |
0.99 |
comp_mlir_nestedIf10 |
10.4221 ms (± 184.928) |
10.5281 ms (± 45.0755) |
0.99 |
comp_mlir_nestedIf100 |
24.967 ms (± 297.347) |
25.0116 ms (± 102.345) |
1.00 |
comp_mlir_chainedIf10 |
9.4924 ms (± 160.407) |
9.56929 ms (± 46.3491) |
0.99 |
comp_mlir_chainedIf100 |
20.21 ms (± 246.736) |
20.2061 ms (± 81.9884) |
1.00 |
comp_cpp_add |
24.7649 ms (± 339.896) |
24.3267 ms (± 189.47) |
1.02 |
comp_cpp_ifThenElse |
25.2519 ms (± 454.938) |
24.9638 ms (± 283.6) |
1.01 |
comp_cpp_deeplyNestedIfElse |
26.3506 ms (± 581.531) |
26.0088 ms (± 233.598) |
1.01 |
comp_cpp_loop |
25.513 ms (± 369.844) |
25.0851 ms (± 291.998) |
1.02 |
comp_cpp_ifInsideLoop |
26.5314 ms (± 548.392) |
26.2072 ms (± 775.414) |
1.01 |
comp_cpp_loopDirectCall |
26.7348 ms (± 1.10497) |
25.3831 ms (± 531.136) |
1.05 |
comp_cpp_pointerLoop |
25.9306 ms (± 428.217) |
25.4858 ms (± 203.254) |
1.02 |
comp_cpp_staticLoop |
25.0875 ms (± 398.141) |
24.7845 ms (± 176.103) |
1.01 |
comp_cpp_fibonacci |
25.6653 ms (± 517.602) |
25.2385 ms (± 205.167) |
1.02 |
comp_cpp_gcd |
25.4064 ms (± 419.127) |
25.0233 ms (± 436.288) |
1.02 |
comp_cpp_nestedIf10 |
28.7362 ms (± 700.036) |
28.0721 ms (± 363.361) |
1.02 |
comp_cpp_nestedIf100 |
61.8357 ms (± 776) |
61.7276 ms (± 707.638) |
1.00 |
comp_cpp_chainedIf10 |
30.8519 ms (± 635.336) |
30.7822 ms (± 574.356) |
1.00 |
comp_cpp_chainedIf100 |
91.8478 ms (± 1.17319) |
91.9107 ms (± 685.096) |
1.00 |
comp_bc_add |
14.5653 us (± 1.90291) |
14.7773 us (± 1.91246) |
0.99 |
comp_bc_ifThenElse |
18.6148 us (± 3.53738) |
19.2429 us (± 3.53172) |
0.97 |
comp_bc_deeplyNestedIfElse |
22.3705 us (± 3.40456) |
23.0339 us (± 3.44593) |
0.97 |
comp_bc_loop |
18.7898 us (± 4.03496) |
18.9128 us (± 2.70016) |
0.99 |
comp_bc_ifInsideLoop |
23.2461 us (± 4.68138) |
21.5194 us (± 2.28647) |
1.08 |
comp_bc_loopDirectCall |
20.1129 us (± 4.15308) |
19.2878 us (± 2.03113) |
1.04 |
comp_bc_pointerLoop |
21.0541 us (± 3.95226) |
20.5351 us (± 2.66896) |
1.03 |
comp_bc_staticLoop |
18.2348 us (± 3.36742) |
17.2132 us (± 2.1155) |
1.06 |
comp_bc_fibonacci |
19.1557 us (± 3.26654) |
19.1709 us (± 2.93634) |
1.00 |
comp_bc_gcd |
18.4041 us (± 3.16772) |
18.5754 us (± 2.5724) |
0.99 |
comp_bc_nestedIf10 |
35.3243 us (± 4.35961) |
35.6002 us (± 4.60432) |
0.99 |
comp_bc_nestedIf100 |
194.196 us (± 8.25529) |
198.41 us (± 10.198) |
0.98 |
comp_bc_chainedIf10 |
49.909 us (± 5.46283) |
52.6073 us (± 9.11991) |
0.95 |
comp_bc_chainedIf100 |
304.809 us (± 16.379) |
309.294 us (± 26.888) |
0.99 |
comp_asmjit_add |
20.9648 us (± 3.07439) |
22.1735 us (± 4.82406) |
0.95 |
comp_asmjit_ifThenElse |
33.1831 us (± 5.50407) |
33.4071 us (± 4.26649) |
0.99 |
comp_asmjit_deeplyNestedIfElse |
56.2223 us (± 9.82083) |
56.4176 us (± 5.25514) |
1.00 |
comp_asmjit_loop |
35.036 us (± 4.90481) |
35.8631 us (± 4.35405) |
0.98 |
comp_asmjit_ifInsideLoop |
57.5714 us (± 10.787) |
57.5183 us (± 8.7641) |
1.00 |
comp_asmjit_loopDirectCall |
45.9792 us (± 8.83067) |
47.2559 us (± 9.00425) |
0.97 |
comp_asmjit_pointerLoop |
48.06 us (± 8.27383) |
48.2785 us (± 6.56048) |
1.00 |
comp_asmjit_staticLoop |
28.4029 us (± 4.62457) |
28.6917 us (± 4.31543) |
0.99 |
comp_asmjit_fibonacci |
43.7309 us (± 7.33849) |
43.5157 us (± 6.92307) |
1.00 |
comp_asmjit_gcd |
35.247 us (± 4.87976) |
35.3711 us (± 4.19816) |
1.00 |
comp_asmjit_nestedIf10 |
101.622 us (± 12.2194) |
103.255 us (± 10.7623) |
0.98 |
comp_asmjit_nestedIf100 |
1052.23 us (± 34766.7) |
1057.97 us (± 54427.5) |
0.99 |
comp_asmjit_chainedIf10 |
153.174 us (± 15.1429) |
153.183 us (± 14.6589) |
1.00 |
comp_asmjit_chainedIf100 |
2.13784 ms (± 42.6684) |
2.16653 ms (± 46.7063) |
0.99 |
tiered_compile_addOne |
55.1822 us (± 13.7359) |
55.1658 us (± 10.7264) |
1.00 |
single_compile_mlir_addOne |
3.20653 ms (± 98.6131) |
3.25749 ms (± 57.5499) |
0.98 |
single_compile_cpp_addOne |
24.8626 ms (± 579.559) |
24.4391 ms (± 405.29) |
1.02 |
single_compile_bc_addOne |
55.7114 us (± 10.6041) |
60.6703 us (± 11.2707) |
0.92 |
tiered_compile_sumLoop |
75.2721 us (± 9.81938) |
79.6202 us (± 15.7788) |
0.95 |
single_compile_mlir_sumLoop |
5.22691 ms (± 119.544) |
5.35632 ms (± 118.42) |
0.98 |
single_compile_cpp_sumLoop |
25.5112 ms (± 501.604) |
26.0899 ms (± 453.281) |
0.98 |
single_compile_bc_sumLoop |
76.4704 us (± 11.9481) |
78.6934 us (± 10.1775) |
0.97 |
ssa_add |
197.912 ns (± 30.1887) |
178.473 ns (± 9.97434) |
1.11 |
ssa_ifThenElse |
512.037 ns (± 47.7256) |
459.53 ns (± 43.4269) |
1.11 |
ssa_deeplyNestedIfElse |
1.24261 us (± 132.58) |
1.16567 us (± 124.837) |
1.07 |
ssa_loop |
533.254 ns (± 37.3753) |
487.861 ns (± 38.2642) |
1.09 |
ssa_ifInsideLoop |
963.3 ns (± 85.7426) |
895.76 ns (± 72.8648) |
1.08 |
ssa_loopDirectCall |
538.595 ns (± 42.1081) |
491.118 ns (± 45.7827) |
1.10 |
ssa_pointerLoop |
651.917 ns (± 86.7096) |
583.073 ns (± 41.8959) |
1.12 |
ssa_staticLoop |
484.438 ns (± 43.463) |
422.539 ns (± 47.7647) |
1.15 |
ssa_fibonacci |
564.952 ns (± 52.841) |
503.737 ns (± 39.1986) |
1.12 |
ssa_gcd |
519.01 ns (± 67.4608) |
445.234 ns (± 36.8343) |
1.17 |
e2e_tiered_bc_to_mlir |
55.854 us (± 13.386) |
56.9167 us (± 7.69512) |
0.98 |
e2e_single_mlir |
5.54266 ms (± 140.045) |
5.52315 ms (± 41.2737) |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
These four files contained only reformatting noise (line rejoining, include reordering) unrelated to the val<bool> value-propagation fix. Restore them to match main so the PR diff is limited to the fix. https://claude.ai/code/session_017PSytSqSqD5DmFBXto3wMr
Adds sumWhileNotDone, a traced function with a loop header of the shape
'while (val<T*> && !val<bool>) { ... *ptr ... }' matching the reported
repro. Calling the JIT'd function with a null pointer must skip the loop
body (returning 0) rather than dereferencing null. This exercises the
val<bool> concrete-value propagation fixed in this PR.
https://claude.ai/code/session_017PSytSqSqD5DmFBXto3wMr
Member
Author
|
Added a regression test ( Heads-up: a separate bytecode-backend gap surfaced by this testWhile validating the test locally I found that the bytecode (
The rest of the I've left the regression test running across all backends so CI can judge it on the real clang-21/MLIR matrix. If Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
val<bool>(TypedValueRef&)constructor which hardcodedvalue = falsefor all traced boolean results, causing incorrect runtime values inval<bool>instances produced by logical ops, comparisons, and pointer comparisonsval(TypedValueRef&, bool actualValue)and use it at all 16 call sites where the actual result is computable:lOr,lAnd,lNot,eq,neq, pointer comparisons (==,!=,<,>,<=,>=), enum comparisons, arithmetic comparisons (viaDEFINE_BINARY_OPERATOR_HELPERmacro), andbool&load operationsContext
When a traced function has a loop header like
while (val<T*> && !val<bool>), the overloaded&&causes the pointer'soperator bool()to go throughtraceBool(), producing a concreteboolthat is re-wrapped as aCONSTin the AND operation. Theval<bool>returned bylNot/ pointer comparisons carriedvalue = falseregardless of the actual result. During control-flow merging on the loop back-edge, this wrong constant persisted in the merged block, effectively removing the pointer null-check from the loop condition and causing a segfault when called with a null pointer.Reported in: https://gist.github.com/ls-1801/541a1d5945cbf700a8dc8664d51fd492
Test plan
cmake --build . --target formatGenerated by Claude Code