On 10/19/21 7:13 PM, Andrew Pinski wrote:
On Tue, Oct 19, 2021 at 3:32 PM Andrew MacLeod <amacl...@redhat.com> wrote:
On 10/19/21 5:13 PM, Andrew Pinski wrote:
On Tue, Oct 19, 2021 at 1:29 PM Andrew MacLeod via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
using testcase ifcvt-4.c:
typedef int word __attribute__((mode(word)));
word
foo (word x, word y, word a)
{
word i = x;
word j = y;
/* Try to make taking the branch likely. */
__builtin_expect (x > y, 1);
if (x > y)
{
i = a;
j = i;
}
return i * j;
The testcase is broken anyways.
The builtin_expect should be inside the if to have any effect. Look
at the estimated values:
if (x_3(D) > y_4(D))
goto <bb 4>; [50.00%] <<-- has been reversed.
else
goto <bb 3>; [50.00%]
;; succ: 4 [50.0% (guessed)] count:536870912 (estimated
locally) (TRUE_VALUE,EXECUTABLE)
;; 3 [50.0% (guessed)] count:536870912 (estimated
locally) (FALSE_VALUE,EXECUTABLE)
See how it is 50/50?
The testcase is not even testing what it says it is testing. Just
happened to work previously does not mean anything. Move the
builtin_expect inside the if and try again. I am shocked it took this
long to find the testcase issue really.
Thanks,
Andrew Pinski
Moving the expect around doesn't change anything, in fact, it makes it
worse since fre and evrp immediately eliminate it as true if it is in
the THEN block.
It looks like it is eliminated by the CDDCE pass:
cddce1 sees:
_1 = x_5(D) > y_7(D);
# RANGE [0, 1] NONZERO 1
_2 = (long int) _1;
__builtin_expect (_2, 1);
if (x_5(D) > y_7(D))
goto <bb 3>; [INV]
else
goto <bb 4>; [INV]
and proceeds:
Marking useful stmt: if (x_5(D) > y_7(D))
processing: if (x_5(D) > y_7(D))
processing: i_3 = PHI <x_5(D)(2), a_9(D)(3)>
Eliminating unnecessary statements:
Deleting : __builtin_expect (_2, 1);
Deleting : _2 = (long int) _1;
Deleting : _1 = x_5(D) > y_7(D);
IF we are suppose to reverse the If, it is not obvious to me who is
suppose to.. You seem to be right that its a crap shot that VRP2 does
it because there isnt enough info to dictate it.. unless somewhere it
detects that a THEN targets an empty block which fallthrus to the ELSE
block should be swapped. Or maybe you are right and that it flukeily
happens due to the ASSERTS being added and removed.
IF i turn of DCE, then this all works like it si ssupopse to.. so maybe
DCE isnt supopse to remove this?
Andrew
