Re: [DTrace-devel] [PATCH 3/3] cg: optimize ternary expressions for strings

[Eugene Loh <eugene.loh@oracle.com>]

Reviewed-by: Eugene Loh <eugene.loh@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/

> 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?

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:

$ 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.