Review request for JDK-8079424: code generator for discarded boolean logical operation has an extra pop

[Attila Szegedi]

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 <sundararajan.athijegannathan at oracle.com> 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.
>