[PATCH] Add if-chain to switch conversion pass.

Michael Matz matz@suse.de
Wed Nov 13 16:14:00 GMT 2019


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.


More information about the Gcc-patches mailing list