[DTrace-devel] [PATCH 3/3] cg: optimize ternary expressions for strings
Kris Van Hees
kris.van.hees at oracle.com
Wed Jul 16 05:32:10 UTC 2025
On Wed, Jul 16, 2025 at 01:00:43AM -0400, Eugene Loh wrote:
> Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
> subject to a few comments below.
>
> On 7/15/25 15:50, Kris Van Hees via DTrace-devel wrote:
>
> > If either size of a ternary expression has a tstring value, it can be
>
> s/size/side/
Thanks.
> > re-used to store the value of the ternary expression, reducing the
> > need for tstring allocation, especially in nested ternaries.
> >
> > diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
> > @@ -222,7 +222,7 @@ typedef struct dt_kern_path {
> > * - cleanpath() holds a prepended '/' char, a string, an appended '/' char,
> > * and a terminating NUL char, or STRSZ + 3 chars altogether
> > */
> > -#define DT_TSTRING_SLOTS 4
> > +#define DT_TSTRING_SLOTS 6
>
> Same comment as I had for Alan's patch. The comment block before this goes
> into excruciating detail about why the value should be 4. Whether its logic
> is right or wrong, we cannot leave those old comments with a new value. Why
> is the value now 6?
Ah yes, I should have updated the explanation. It has become a little bit more
complicated but I can certainly explain it based on the example we have in the
testsuite.
>
> I assume 6 is not a "sufficient for all purposes" value. E.g., if I kick up
> the complexity on tst.tstring_ternary_nested.d, I get:
Correct - I hope to do a later patch that makes this something that adjusts
dynamically. But that is quite a bit more work.
>
> $ git diff
> diff --git a/test/unittest/codegen/tst.tstring_ternary_nested.d
> b/test/unittest/codegen/tst.tstring_ternary_nested.d
> @@ -16,6 +16,8 @@ BEGIN {
> trace(x > 1 ? strjoin(strjoin("a", "bc"), strjoin("de", "f")) :
> x > 2 ? strjoin(strjoin("A", "BC"), strjoin("DE", "F")) :
> x > 3 ? strjoin(strjoin("u", "vw"), strjoin("xy", "z")) :
> + x > 4 ? strjoin(strjoin("u", "vw"), strjoin("xy", "z")) :
> + x > 5 ? strjoin(strjoin("u", "vw"), strjoin("xy", "z")) :
> strjoin(strjoin("U", "VW"), strjoin("XY", "Z")));
>
> exit(0);
> $ sudo ./runtest.sh test/unittest/codegen/tst.tstring_ternary_nested.d
> test/unittest/codegen/tst.tstring_ternary_nested.d: FAIL: core dumped.
> 1 cases (0 PASS, 1 FAIL, 0 XPASS, 0 XFAIL, 0 SKIP)
> $ cat test/log/current/runtest.log
> [...]
> dtrace: libdtrace/dt_cg.c:1472: dt_cg_tstring_xalloc: Assertion `i <
> DT_TSTRING_SLOTS' failed.
>
> > diff --git a/test/unittest/codegen/tst.tstring_ternary_nested.d b/test/unittest/codegen/tst.tstring_ternary_nested.d
> > @@ -0,0 +1,26 @@
> > +BEGIN {
> > + x = 42;
> > + trace(x > 1 ? strjoin(strjoin("a", "bc"), strjoin("de", "f")) :
> > + x > 2 ? strjoin(strjoin("A", "BC"), strjoin("DE", "F")) :
> > + x > 3 ? strjoin(strjoin("u", "vw"), strjoin("xy", "z")) :
> > + strjoin(strjoin("U", "VW"), strjoin("XY", "Z")));
> > +
> > + exit(0);
> > +}
>
> I guess that's fine for cg, but the skeptic in me notices that the three
> tests all evaluate to true. I would think one could run through all 2x2x2=8
> cases in a single test for a little bit more rigor.
Since this is a codegen test, we only really care about the tstring allocation
aspect. There are already other tests that exercise the ternary itself.
More information about the DTrace-devel
mailing list