Hi,
On Wed, 13 Nov 2019, Martin Liška wrote:
> > Not a review, just a few questions:
>
> Hello.
>
> Thank you for it.
>
> >
> > 1) what does it do if __builtin_expect* has been used, does it preserve
> > the probabilities and if in the end decides to expand as ifs, are those
> > probabilities retained through it?
>
> No, it's currently not supported. I can consider adding that as a follow up.
But given that you apply the transformation even to small if ladders this
looses quite some info then.
> > 2) for the reassoc-*.c testcases, do you get identical or better code
> > with the patch?
>
> The code is the following:
>
> Before:
> Optimizing range tests a_5(D) -[10, 10] and -[26, 26]
> into (a_5(D) & -17) != 10
>
> <bb 2> [local count: 1073741823]:
> _11 = a_5(D) & -17;
> _12 = _11 == 10;
> _1 = a_5(D) == 10;
> _2 = a_5(D) == 12;
> _10 = a_5(D) == 26;
> _9 = _2 | _12;
> if (_9 != 0)
> goto <bb 4>; [56.44%]
> else
> goto <bb 3>; [43.56%]
>
> After:
>
> <bb 2> [local count: 1073741824]:
> switch (a_2(D)) <default: <L5> [50.00%], case 10: <L6> [50.00%], case 12:
> <L6> [50.00%], case 26: <L6> [50.00%]>
And here I bet that Jakub was asking for final code, not intermediate
code. In particular the bit trick with transforming a test for
a \in {10,26} into (a&-17)==10. The switch still tests for 10,26, and in
the end it should be ensured that the same trickery is employed to
actually implement the switch.
> As seen, reassoc can generate a different range check and then it's about cmp1
> | cmp2 | ..
> I bet the explicit gswitch is quite equal representation.
As long as the final code is the same or better, sure. Otherwise it's a
more canonical representation with suboptimal expansion, and the latter
should be fixed otherwise it's introducing a regression.
Ciao,
Michael.
