« first day (4967 days earlier)      last day (203 days later) » 

07:43
One of these "class_exists()"s returns false.
None of them should.
```php
<?php
$alice = Alice::class;
$bob = Bob::class;
var_dump(class_exists($alice));
var_dump(class_exists($bob));
class Alice {}
class Bob implements myInt {}
var_dump(class_exists($alice));
var_dump(class_exists($bob));
interface myInt {}
```

I need to reactivate the Github account that I abandoned 8 years ago in order to report a bug.
And the 3v4l for sauce... 3v4l.org/ESR4S
@LeviMorrison zend_ulong h = (ZSTR_H(a) ^ ZSTR_H(b)) * ZSTR_H(a) * ZSTR_H(b); would be a branchless check for hash(a) == hash(b) && hash(a) != 0 && hash(b) != 0.
08:03
@Derick I think the setup-php-sdk action has an issue - I'm getting "toolset not found" errors for PHP 7.4 since yesterday. Does 7.4 work for you?
 
6 hours later…
13:36
@alcaeus just opened github.com/php/setup-php-sdk/pull/1 to advise users to use windows-2019 for 7.4 and earlier. I'm still looking into how we can manually install vc15, since we'll need windows-2022 for the rest of ext-mongodb's release process.
 
1 hour later…
14:59
@TimWolla In the situation, I think it needs to be hash(a) == hash(b) || hash(a)==0 || hash(b) == 0... right? Because if it's zero we need it to be "true", rather than false.
@LeviMorrison Whoops, sorry. It should have read hash(a) != hash(b), not ==.
We want to check whether the hashes are different and both != 0, then the strings are unequal.
I think I flubbed mine just now too :D
if ((hash(a) != hash(b) && hash(a) != 0 && hash(b) != 0)
    || (len(a) != len(b))) {
    return false;
}
Yes.
If the branchless version of that has reasonable cost, then sure, it could be worth doing. But we'll have to see. It's worth it if you can rely on the hashes already being set (such as if you are using these in hashtables, and effectively the hashtable already does this, though it doesn't make a named function for it).
@LeviMorrison Talking removing branches: github.com/php/php-src/pull/14108#discussion_r1587084654, that is not technically correct, because zend_is_true returns int.
It's also compiled into a cmov anyways.
Corrected link with fixed return type: godbolt.org/z/cva7Mccjf
15:13
It's a bool cast to an int, so that's going to be exactly 0 or 1, yes?
I'm suspicious of conditional assignments, I've seen the compilers be inconsistent about doing cmov for them over the years.
@LeviMorrison Does that also hold in face of compiler optimizations and on all architectures? For this kind of trick I'm always suspicious, because UB is a thing :-D
I'd rather write code that is obviously correct and let the compiler writer do the hard work.
I'm pretty sure bool to int is required to be 0 or 1. I can try to look it up.
Perhaps the correct fix would be changing zend_is_true to just return bool.
Yes, that should have been done when the impl it calls was switched to bool ^_^
58
A: Is bool guaranteed to be 0 or 1 when converted to int?

Matthew FlaschenYes: In C++ (§4.5/4): An rvalue of type bool can be converted to an rvalue of type int, with false becoming zero and true becoming one. In C, when a value is converted to _Bool, it becomes 0 or 1 (§6.3.1.2/1): When any scalar value is converted to _Bool, the result is 0 if the ...

^ Not an authoritative source but they quoted enough we could look it up if we really cared :D
@TimWolla Want me to open a PR for that? Or do you have other return type conversions you want to do in a batch?
@LeviMorrison I don't have anything else, no. And yes, feel free to PR that.
If types are cleaned up, then I'd update the suggestion to a zend_is_true(…) != negate_condition;, instead of ^, though. Using the boolean != feels more correct than XOR.
15:21
Gotcha. The patch could probably just use i_zend_is_true too.
@LeviMorrison That one is force inlined. In the case of array_find() that probably needlessly increases assembly size.
Most of the time is spent calling the user callback anyways, no need to waste precious instruction cache space for the zend_is_true.
The array_find internals will likely get used by a few functions in the final implementation.
In such a case, I probably wouldn't care about any space differences--the biggest space savings would be reusing those internals for the 2-4 functions it powers.
Using the force-inlined variant without measuring feels like premature optimization. In fact I'm not even sure if the branch-less negation would even be measurably faster. The branch is easily predicted.
Reducing branches in cases like this allows for better utilization of the branch predictor's internal resources, because it no longer needs any since there aren't any branches.
And in this case, there's not really a big cost. Sometimes branchless is possible but comparatively more expensive, and in those cases, yes, you definitely want to micro-bench that. But this one is trivial to eliminate.
As an example, I'm not sure a branchless version of this one would be worth it: chat.stackoverflow.com/transcript/message/57376030#57376030.
15:36
Yeah, as I said, I generally trust the compiler more than myself :-D
15:50
child = &ast->child[2 - zend_is_true(zend_ast_get_zval(ast->child[0]))];
This is relying on it being exactly 0 or 1 already. This is the "weirdest" usage I've found so far and it would definitely prefer bool to int.
16:01
typedef struct _php_bz2_filter_data {
	bz_stream strm;
	char *inbuf;
	char *outbuf;
	size_t inbuf_len;
	size_t outbuf_len;

	enum strm_status status;              /* Decompress option */
	unsigned int small_footprint : 1;     /* Decompress option */
	unsigned int expect_concatenated : 1; /* Decompress option */
	unsigned int is_flushed : 1;          /* only for compression */

	int persistent;
} php_bz2_filter_data;
^ it gets stored into these too.
Ironically, I don't think those are saving any total bytes. There are only 3 of them, and 3 bools take less size than an unsigned int does (on our target platforms, anyway)
 
2 hours later…
17:51
Is there any way to get header() & headers_list() to work under CLI (PHPUnit)?
18:06
@QuolonelQuestions The genuine answer is... don't use them directly
You could potentially use an extension to replace the underlying functions, but using something like symfony http kernel would be a heck of a lot better
18:43
@LeviMorrison I'd definitely write that with an explicit ternary and rely on the compiler to simplify that :-D
Not sure about this one:
ref = ir_CALL_1(IR_BOOL, ir_CONST_FC_FUNC(zend_is_true), jit_ZVAL_ADDR(jit, op1_addr));
I'll call it out in the PR.
Someone really needs to ban macros from the codebase.
(assuming those are macros, given the uppercase naming)
19:01
@TimWolla Something something JIT compiler and register
As I was doing the migration for that and I was getting weird failures
19:24
Oh fun, we have it used in a multiply too:
sockopts |= STREAM_SOCKOP_IPV6_V6ONLY_ENABLED * zend_is_true(tmpzval);
Just audited everything in php-src. The only one I don't know how to fix is the JIT IR one.
@LeviMorrison Welcome to my issue :D
I'm going to open a PR and try and figure it out. It's R&D week for me and I do get JIT failures in our CI from time to time, so as part of R, I can learn a smidge more about it while improving something else.
 
1 hour later…
20:38
PR open, awaiting tests. Seemed to work locally (although 3 failed, still investigating them, but I didn't run this without patches so they may have failed anyway): github.com/php/php-src/pull/14301.
 
1 hour later…
21:39
@LeviMorrison Thank you. It's interesting to note that the code in Zend/Optimizer/pass1.c is very similar to that of array_find.
If that uses an explicit branch, it probably makes sense for array_find to do the same (and vice versa).
@LeviMorrison the JIT compiler doesn't care about the specific return value of the functions it consumes… you are basically required to ensure that it returns something which is returned the standard return value register (which is fine for bool, but you shouldn't return a bare (non-pointer) zval in a JIT function)

« first day (4967 days earlier)      last day (203 days later) »