Merge switch statements in tree-cfgcleanup

Jeff Law law@redhat.com
Wed Jul 20 16:09:00 GMT 2016


On 07/20/2016 05:14 AM, Bernd Schmidt wrote:
> On 07/19/2016 01:18 PM, Richard Biener wrote:
>> On Tue, Jul 19, 2016 at 1:07 PM, Bernd Schmidt
>> <bschmidt@redhat.com> wrote:
>>> On 07/19/2016 12:35 PM, Richard Biener wrote:
>>>
>>>> I think that start/end_recording_case_labels also merged
>>>> adjacent labels via group_case_labels_stmt.  Not sure why you
>>>> need to stop recording case labels during the transform.  Is
>>>> this because you are building a new switch stmt?
>>>
>>>
>>> It's because the cached mapping gets invalidated. Look in
>>> tree-cfg, it has a edge_to_cases map which I think cannot be
>>> maintained if you modify the structure. I certainly got lots of
>>> internal errors until I added that pair of calls.
>>
>> Yeah, I see that.  OTOH cfgcleanup relies on this cache to be
>> efficient and you (repeatedly) clear it.  Clearing parts of it
>> should be sufficient and if you used redirect_edge_and_branch
>> instead of redirect_edge_pred it would have maintained the cache as
>> far as I can see,
>
> I don't think that would work, since we're modifying and/or
> discarding case labels as well and they can't remain part of the
> cache.
>
>> or you can make sure to maintain it yourself or just clear the info
>> associated with the edges you redirect from one switch to another.
>
> How's this? Tested as before.
So I'm going to let Richi run with the review on this one since the two 
of you are already iterating.  But I did have one comment on the 
placement of the pass.

I believe one of the key things to consider for whether or not something 
like this belongs in the cfgcleanup code is whether or not the 
optimization is likely exposed repeatedly through the optimization 
pipeline.  If it's mostly a source level issue or only usually exposed 
by a limited set of optimizers, then a separate pass might be better.



jeff



More information about the Gcc-patches mailing list