[PATCH] Fix emission of exception dispatch (PR middle-end/82154).

[Jeff Law]

On 09/13/2017 12:09 PM, Martin Liška wrote:
> On 09/13/2017 04:22 PM, Jeff Law wrote:
>> On 09/13/2017 07:42 AM, Martin Liška wrote:
>>> On 09/13/2017 03:08 PM, Martin Liška wrote:
>>>> On 09/12/2017 05:21 PM, Jeff Law wrote:
>>>>> On 09/12/2017 01:43 AM, Martin Liška wrote:
>>>>>> Hello.
>>>>>>
>>>>>> In transition to simple_case_node, I forgot to initialize m_high to m_low if a case
>>>>>>
>>>>>> does not have CASE_HIGH.
>>>>>>
>>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>>>>
>>>>>>
>>>>>> Ready to be installed?
>>>>>> Martin
>>>>>>
>>>>>> gcc/ChangeLog:
>>>>>>
>>>>>> 2017-09-11  Martin Liska  <mliska@suse.cz>
>>>>>>
>>>>>>     PR middle-end/82154
>>>>>>     * stmt.c (struct simple_case_node): Assign low to high when
>>>>>>     high is equal to null.
>>>>>>
>>>>>> gcc/testsuite/ChangeLog:
>>>>>>
>>>>>> 2017-09-11  Martin Liska  <mliska@suse.cz>
>>>>>>
>>>>>>     PR middle-end/82154
>>>>>>     * g++.dg/torture/pr82154.C: New test.
>>>>> OK.
>>>>>
>>>>> THough I have to wonder if we should unify the HIGH handling -- having
>>>>> two different conventions is bound to be confusing.
>>>>>
>>>>> In a CASE_LABEL_EXPR the CASE_HIGH can be NULL, which means the label
>>>>> refers to a singleton value that is found in CASE_LOW.
>>>>>
>>>>> That means we end up doing stuff like this:
>>>>>
>>>>>   if (CASE_HIGH (elt))
>>>>>      maxval = fold_convert (index_type, CASE_HIGH (elt));
>>>>>    else
>>>>>      maxval = fold_convert (index_type, CASE_LOW (elt));
>>>>>
>>>>>
>>>>>
>>>>> You could legitimately argue for changing how this works for tree nodes
>>>>>
>>>>> so that there's always a non-null CASE_HIGH.
>>>>
>>>> Hi.
>>>>
>>>> Agree with you that we have a lot of code that does what you just described.
>>>>
>>>> I tent to change IL representation, where CASE_HIGH is always non-null.
>>>>
>>>> $ git grep 'CASE_HIGH\>' | wc -l
>>>> 125
>>>>
>>>> Which is reasonable amount of code that should be changed. I'll prepare patch for that.
>>>>
>>>
>>> Hm, there's one question that pops up: Should we compare the values via pointer equality,
>>>
>>> or tree_int_cst_eq should be done? The later one can messy the code a bit.
>>>
>> I'd like to say pointer equality, but it's probably safer to use
>> tree_int_cst_eq.
>>
>> jeff
>>
> 
> Ok, so I'll put it on my TODO list as it will take some time to do the transformation.
NP.  There's nothing inherently wrong with the code today, it's just a
cleanup (I hope :-)

> 
> 
> In order to fix the PR, I suggest following patch.
> 
> Ready for trunk?
> Martin
> 
> 0001-Fix-emission-of-exception-dispatch-PR-middle-end-821.patch
> 
> 
> From 1cd8987ad3de4a7bd3e71014a0df9ccb3be40277 Mon Sep 17 00:00:00 2001
> From: marxin <mliska@suse.cz>
> Date: Mon, 11 Sep 2017 13:34:41 +0200
> Subject: [PATCH] Fix emission of exception dispatch (PR middle-end/82154).
> 
> gcc/ChangeLog:
> 
> 2017-09-11  Martin Liska  <mliska@suse.cz>
> 
> 	PR middle-end/82154
> 	* stmt.c (expand_sjlj_dispatch_table): Use CASE_LOW when
> 	CASE_HIGH is NULL_TREE.
> 
> gcc/testsuite/ChangeLog:
> 
> 2017-09-11  Martin Liska  <mliska@suse.cz>
> 
> 	PR middle-end/82154
> 	* g++.dg/torture/pr82154.C: New test.
OK.

Jeff