What mattered to produce the bug was whether the result of a boolean expression
was discarded or not. In <expr1>,<expr2> comma-expression expr1 is discarded;
so is expr in void <expr>. I did add tests originally for non-boolean (numeric)
operands too though, to make sure the other path in that method (for
non-boolean expression) also doesn’t contain a similar bug.
What’s funny is that non-boolean actually can evaluate to smaller (albeit still
non-zero) code than boolean. E.g.
void (true && true)
becomes
iconst_1
ifeq L3
iconst_1
ifeq L3
L3: ...
while
void (1 && 1)
becomes:
iconst_1
ifeq L3
L3: …
This is because boolean evaluation uses branch optimizer that doesn’t have
“should this be discarded” logic, while non-boolean evaluation can notice that
the second operand is to be discarded, and we’re good at eliminating code for
discarded constants :-) Obviously, I didn’t want to go in there and start
implementing one. We can obviously be even better at dead code elimination if
we want to, but we need to draw a line somewhere between extra code for it in
CodeGenerator (and extra runtime to execute it whenever code is generated) and
the payoff, namely that most of the time people hopefully don’t even write code
as braindead as this (using boolean operators on constant expressions that are
either always true or false), so why pay attention to it.
(On the other hand, *of course* I want to make the compiler smart enough to
eliminate this to 0 instructions emitted… oh well).
Attila.
> On May 10, 2015, at 10:17 PM, A. Sundararajan
> <[email protected]> wrote:
>
> I recall original test reported used comma operator as well - like true &&
> false, true. Test could be expanded to include few expressions with comma
> operators as well.
>
> Other than that, +1
>
> -Sundar
>
> On Sunday 10 May 2015 04:14 PM, Attila Szegedi wrote:
>> Please review JDK-8079424 "code generator for discarded boolean logical
>> operation has an extra pop" at
>> <http://cr.openjdk.java.net/~attila/8079424/webrev.00> for
>> <https://bugs.openjdk.java.net/browse/JDK-8079424>
>>
>> Thanks,
>> Attila.
>
