diff mbox series

Restore input_location after recursive expand_call_inline

Message ID AM0PR0602MB3410D0C853878A494D56E5D5E4D20@AM0PR0602MB3410.eurprd06.prod.outlook.com
State New
Headers show
Series Restore input_location after recursive expand_call_inline | expand

Commit Message

Bernd Edlinger Jan. 4, 2021, 8:12 p.m. UTC 
Hi,

I spotted a place where input_location is clobbered accidentally.

That is in a recursive call to expand_call_inline.  The input_location
is usually restored by goto egress in this function.

Additionally the return value of the recursive expand call is thrown
away, which does not look like a good idea.

Although this causes no problems ATM, I wanted to fix it anyway.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.

Comments

Jeff Law Jan. 4, 2021, 9:23 p.m. UTC | #1 
On 1/4/21 1:12 PM, Bernd Edlinger wrote:
> Hi,
>
> I spotted a place where input_location is clobbered accidentally.
>
> That is in a recursive call to expand_call_inline.  The input_location
> is usually restored by goto egress in this function.
>
> Additionally the return value of the recursive expand call is thrown
> away, which does not look like a good idea.
>
> Although this causes no problems ATM, I wanted to fix it anyway.
>
>
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
>
>
> Thanks
> Bernd.
>
> 0001-Restore-input_location-after-recursive-expand_call_i.patch
>
> From 88b963bba7b32972abf0ea44a01c03d643d7c6ca Mon Sep 17 00:00:00 2001
> From: Bernd Edlinger <bernd.edlinger@hotmail.de>
> Date: Mon, 4 Jan 2021 11:35:31 +0100
> Subject: [PATCH] Restore input_location after recursive expand_call_inline
>
> This is just a precautionary fix.
>
> 2021-01-04  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>
> 	* tree-inline.c (expand_call_inline): Restore input_location.
> 	Return result from recursive call.
I suspect that we're always supposed to inline in this case.  As
asserting that successfully_inlined is true before jumping to "egress"
seems wise.

OK with that change after the usual testing.

Jeff
Bernd Edlinger Jan. 5, 2021, 6:44 a.m. UTC | #2 
On 1/4/21 10:23 PM, Jeff Law wrote:
> 
> 
> On 1/4/21 1:12 PM, Bernd Edlinger wrote:
>> Hi,
>>
>> I spotted a place where input_location is clobbered accidentally.
>>
>> That is in a recursive call to expand_call_inline.  The input_location
>> is usually restored by goto egress in this function.
>>
>> Additionally the return value of the recursive expand call is thrown
>> away, which does not look like a good idea.
>>
>> Although this causes no problems ATM, I wanted to fix it anyway.
>>
>>
>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk?
>>
>>
>> Thanks
>> Bernd.
>>
>> 0001-Restore-input_location-after-recursive-expand_call_i.patch
>>
>> From 88b963bba7b32972abf0ea44a01c03d643d7c6ca Mon Sep 17 00:00:00 2001
>> From: Bernd Edlinger <bernd.edlinger@hotmail.de>
>> Date: Mon, 4 Jan 2021 11:35:31 +0100
>> Subject: [PATCH] Restore input_location after recursive expand_call_inline
>>
>> This is just a precautionary fix.
>>
>> 2021-01-04  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>
>> 	* tree-inline.c (expand_call_inline): Restore input_location.
>> 	Return result from recursive call.
> I suspect that we're always supposed to inline in this case.  As
> asserting that successfully_inlined is true before jumping to "egress"
> seems wise.
> 
> OK with that change after the usual testing.
> 

No this does not work:

+FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++98 (internal compiler error)
+FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++98 (test for excess errors)
+UNRESOLVED: g++.dg/ipa/devirt-5.C  -std=gnu++98 compilation failed to produce executable
+FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++14 (internal compiler error)
+FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++14 (test for excess errors)
+UNRESOLVED: g++.dg/ipa/devirt-5.C  -std=gnu++14 compilation failed to produce executable
+FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++17 (internal compiler error)
+FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++17 (test for excess errors)
+UNRESOLVED: g++.dg/ipa/devirt-5.C  -std=gnu++17 compilation failed to produce executable
+FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++2a (internal compiler error)
+FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++2a (test for excess errors)
+UNRESOLVED: g++.dg/ipa/devirt-5.C  -std=gnu++2a compilation failed to produce executable
+FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++98 (internal compiler error)
+FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++98 (test for excess errors)
+UNRESOLVED: g++.dg/ipa/devirt-c-4.C  -std=gnu++98 compilation failed to produce executable
+FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++14 (internal compiler error)
+FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++14 (test for excess errors)
+UNRESOLVED: g++.dg/ipa/devirt-c-4.C  -std=gnu++14 compilation failed to produce executable
+FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++17 (internal compiler error)
+FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++17 (test for excess errors)
+UNRESOLVED: g++.dg/ipa/devirt-c-4.C  -std=gnu++17 compilation failed to produce executable
+FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++2a (internal compiler error)
+FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++2a (test for excess errors)
+UNRESOLVED: g++.dg/ipa/devirt-c-4.C  -std=gnu++2a compilation failed to produce executable
+FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++98 (internal compiler error)
+FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++98 (test for excess errors)
+UNRESOLVED: g++.dg/ipa/imm-devirt-2.C  -std=gnu++98 compilation failed to produce executable
+FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++14 (internal compiler error)
+FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++14 (test for excess errors)
+UNRESOLVED: g++.dg/ipa/imm-devirt-2.C  -std=gnu++14 compilation failed to produce executable
+FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++17 (internal compiler error)
+FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++17 (test for excess errors)
+UNRESOLVED: g++.dg/ipa/imm-devirt-2.C  -std=gnu++17 compilation failed to produce executable
+FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++2a (internal compiler error)
+FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++2a (test for excess errors)
+UNRESOLVED: g++.dg/ipa/imm-devirt-2.C  -std=gnu++2a compilation failed to produce executable
+FAIL: g++.dg/ipa/pr71146.C  -std=gnu++98 (internal compiler error)
+FAIL: g++.dg/ipa/pr71146.C  -std=gnu++98 (test for excess errors)
+FAIL: g++.dg/ipa/pr71146.C  -std=gnu++14 (internal compiler error)
+FAIL: g++.dg/ipa/pr71146.C  -std=gnu++14 (test for excess errors)
+FAIL: g++.dg/ipa/pr71146.C  -std=gnu++17 (internal compiler error)
+FAIL: g++.dg/ipa/pr71146.C  -std=gnu++17 (test for excess errors)
+FAIL: g++.dg/ipa/pr71146.C  -std=gnu++2a (internal compiler error)
+FAIL: g++.dg/ipa/pr71146.C  -std=gnu++2a (test for excess errors)
+FAIL: g++.dg/ipa/pr79776.C  -std=gnu++98 (internal compiler error)
+FAIL: g++.dg/ipa/pr79776.C  -std=gnu++98 (test for excess errors)
+FAIL: g++.dg/ipa/pr79776.C  -std=gnu++14 (internal compiler error)
+FAIL: g++.dg/ipa/pr79776.C  -std=gnu++14 (test for excess errors)
+FAIL: g++.dg/ipa/pr79776.C  -std=gnu++17 (internal compiler error)
+FAIL: g++.dg/ipa/pr79776.C  -std=gnu++17 (test for excess errors)
+FAIL: g++.dg/ipa/pr79776.C  -std=gnu++2a (internal compiler error)
+FAIL: g++.dg/ipa/pr79776.C  -std=gnu++2a (test for excess errors)
+FAIL: g++.dg/ipa/pr85421.C   (internal compiler error)
+FAIL: g++.dg/ipa/pr85421.C   (test for excess errors)
+FAIL: g++.dg/ipa/pr91969.C  -std=gnu++98 (internal compiler error)
+FAIL: g++.dg/ipa/pr91969.C  -std=gnu++98 (test for excess errors)
+FAIL: g++.dg/ipa/pr91969.C  -std=gnu++14 (internal compiler error)
+FAIL: g++.dg/ipa/pr91969.C  -std=gnu++14 (test for excess errors)
+FAIL: g++.dg/ipa/pr91969.C  -std=gnu++17 (internal compiler error)
+FAIL: g++.dg/ipa/pr91969.C  -std=gnu++17 (test for excess errors)
+FAIL: g++.dg/ipa/pr91969.C  -std=gnu++2a (internal compiler error)
+FAIL: g++.dg/ipa/pr91969.C  -std=gnu++2a (test for excess errors)
+FAIL: g++.dg/ipa/pr92454.C  -std=gnu++98 (internal compiler error)
+FAIL: g++.dg/ipa/pr92454.C  -std=gnu++98 (test for excess errors)
+FAIL: g++.dg/ipa/pr92454.C  -std=gnu++14 (internal compiler error)
+FAIL: g++.dg/ipa/pr92454.C  -std=gnu++14 (test for excess errors)
+FAIL: g++.dg/ipa/pr92454.C  -std=gnu++17 (internal compiler error)
+FAIL: g++.dg/ipa/pr92454.C  -std=gnu++17 (test for excess errors)
+FAIL: g++.dg/ipa/pr92454.C  -std=gnu++2a (internal compiler error)
+FAIL: g++.dg/ipa/pr92454.C  -std=gnu++2a (test for excess errors)
 FAIL: g++.dg/guality/pr55665.C   -O2 -flto -fno-use-linker-plugin -flto-partition=none  line 23 p == 40
+FAIL: g++.dg/lto/devirt-5 cp_lto_devirt-5_0.o-cp_lto_devirt-5_0.o link, -O3 -fno-early-inlining -fno-inline -fdump-ipa-cp -fdump-tree-optimized -flto (internal compiler error)

Is it OK in the original form?


Thanks
Bernd.


> Jeff
>
Richard Biener Jan. 5, 2021, 8:05 a.m. UTC | #3 
On Tue, 5 Jan 2021, Bernd Edlinger wrote:

> 
> 
> On 1/4/21 10:23 PM, Jeff Law wrote:
> > 
> > 
> > On 1/4/21 1:12 PM, Bernd Edlinger wrote:
> >> Hi,
> >>
> >> I spotted a place where input_location is clobbered accidentally.
> >>
> >> That is in a recursive call to expand_call_inline.  The input_location
> >> is usually restored by goto egress in this function.
> >>
> >> Additionally the return value of the recursive expand call is thrown
> >> away, which does not look like a good idea.
> >>
> >> Although this causes no problems ATM, I wanted to fix it anyway.
> >>
> >>
> >> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> >> Is it OK for trunk?
> >>
> >>
> >> Thanks
> >> Bernd.
> >>
> >> 0001-Restore-input_location-after-recursive-expand_call_i.patch
> >>
> >> From 88b963bba7b32972abf0ea44a01c03d643d7c6ca Mon Sep 17 00:00:00 2001
> >> From: Bernd Edlinger <bernd.edlinger@hotmail.de>
> >> Date: Mon, 4 Jan 2021 11:35:31 +0100
> >> Subject: [PATCH] Restore input_location after recursive expand_call_inline
> >>
> >> This is just a precautionary fix.
> >>
> >> 2021-01-04  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> >>
> >> 	* tree-inline.c (expand_call_inline): Restore input_location.
> >> 	Return result from recursive call.
> > I suspect that we're always supposed to inline in this case.  As
> > asserting that successfully_inlined is true before jumping to "egress"
> > seems wise.
> > 
> > OK with that change after the usual testing.
> > 
> 
> No this does not work:
> 
> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++98 (internal compiler error)
> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++98 (test for excess errors)
> +UNRESOLVED: g++.dg/ipa/devirt-5.C  -std=gnu++98 compilation failed to produce executable
> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++14 (internal compiler error)
> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++14 (test for excess errors)
> +UNRESOLVED: g++.dg/ipa/devirt-5.C  -std=gnu++14 compilation failed to produce executable
> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++17 (internal compiler error)
> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++17 (test for excess errors)
> +UNRESOLVED: g++.dg/ipa/devirt-5.C  -std=gnu++17 compilation failed to produce executable
> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++2a (internal compiler error)
> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++2a (test for excess errors)
> +UNRESOLVED: g++.dg/ipa/devirt-5.C  -std=gnu++2a compilation failed to produce executable
> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++98 (internal compiler error)
> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++98 (test for excess errors)
> +UNRESOLVED: g++.dg/ipa/devirt-c-4.C  -std=gnu++98 compilation failed to produce executable
> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++14 (internal compiler error)
> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++14 (test for excess errors)
> +UNRESOLVED: g++.dg/ipa/devirt-c-4.C  -std=gnu++14 compilation failed to produce executable
> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++17 (internal compiler error)
> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++17 (test for excess errors)
> +UNRESOLVED: g++.dg/ipa/devirt-c-4.C  -std=gnu++17 compilation failed to produce executable
> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++2a (internal compiler error)
> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++2a (test for excess errors)
> +UNRESOLVED: g++.dg/ipa/devirt-c-4.C  -std=gnu++2a compilation failed to produce executable
> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++98 (internal compiler error)
> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++98 (test for excess errors)
> +UNRESOLVED: g++.dg/ipa/imm-devirt-2.C  -std=gnu++98 compilation failed to produce executable
> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++14 (internal compiler error)
> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++14 (test for excess errors)
> +UNRESOLVED: g++.dg/ipa/imm-devirt-2.C  -std=gnu++14 compilation failed to produce executable
> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++17 (internal compiler error)
> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++17 (test for excess errors)
> +UNRESOLVED: g++.dg/ipa/imm-devirt-2.C  -std=gnu++17 compilation failed to produce executable
> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++2a (internal compiler error)
> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++2a (test for excess errors)
> +UNRESOLVED: g++.dg/ipa/imm-devirt-2.C  -std=gnu++2a compilation failed to produce executable
> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++98 (internal compiler error)
> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++98 (test for excess errors)
> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++14 (internal compiler error)
> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++14 (test for excess errors)
> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++17 (internal compiler error)
> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++17 (test for excess errors)
> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++2a (internal compiler error)
> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++2a (test for excess errors)
> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++98 (internal compiler error)
> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++98 (test for excess errors)
> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++14 (internal compiler error)
> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++14 (test for excess errors)
> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++17 (internal compiler error)
> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++17 (test for excess errors)
> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++2a (internal compiler error)
> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++2a (test for excess errors)
> +FAIL: g++.dg/ipa/pr85421.C   (internal compiler error)
> +FAIL: g++.dg/ipa/pr85421.C   (test for excess errors)
> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++98 (internal compiler error)
> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++98 (test for excess errors)
> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++14 (internal compiler error)
> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++14 (test for excess errors)
> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++17 (internal compiler error)
> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++17 (test for excess errors)
> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++2a (internal compiler error)
> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++2a (test for excess errors)
> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++98 (internal compiler error)
> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++98 (test for excess errors)
> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++14 (internal compiler error)
> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++14 (test for excess errors)
> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++17 (internal compiler error)
> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++17 (test for excess errors)
> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++2a (internal compiler error)
> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++2a (test for excess errors)
>  FAIL: g++.dg/guality/pr55665.C   -O2 -flto -fno-use-linker-plugin -flto-partition=none  line 23 p == 40
> +FAIL: g++.dg/lto/devirt-5 cp_lto_devirt-5_0.o-cp_lto_devirt-5_0.o link, -O3 -fno-early-inlining -fno-inline -fdump-ipa-cp -fdump-tree-optimized -flto (internal compiler error)
> 
> Is it OK in the original form?

Can you add a comment like

  /* This used to return true even though we do fail to inline in
     some cases.  See PRXYZ.  */

and file a bug?

OK with that change.

Some RAII-style auto_input_location class might be useful -
temporarily setting input_location for some "legacy" code is
the only valid use of input_location in the middle-end.

Thanks,
Richard.

> 
> Thanks
> Bernd.
> 
> 
> > Jeff
> > 
>
Jeff Law Jan. 5, 2021, 4:51 p.m. UTC | #4 
On 1/5/21 1:05 AM, Richard Biener wrote:
> On Tue, 5 Jan 2021, Bernd Edlinger wrote:
>
>>
>> On 1/4/21 10:23 PM, Jeff Law wrote:
>>>
>>> On 1/4/21 1:12 PM, Bernd Edlinger wrote:
>>>> Hi,
>>>>
>>>> I spotted a place where input_location is clobbered accidentally.
>>>>
>>>> That is in a recursive call to expand_call_inline.  The input_location
>>>> is usually restored by goto egress in this function.
>>>>
>>>> Additionally the return value of the recursive expand call is thrown
>>>> away, which does not look like a good idea.
>>>>
>>>> Although this causes no problems ATM, I wanted to fix it anyway.
>>>>
>>>>
>>>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>>>> Is it OK for trunk?
>>>>
>>>>
>>>> Thanks
>>>> Bernd.
>>>>
>>>> 0001-Restore-input_location-after-recursive-expand_call_i.patch
>>>>
>>>> From 88b963bba7b32972abf0ea44a01c03d643d7c6ca Mon Sep 17 00:00:00 2001
>>>> From: Bernd Edlinger <bernd.edlinger@hotmail.de>
>>>> Date: Mon, 4 Jan 2021 11:35:31 +0100
>>>> Subject: [PATCH] Restore input_location after recursive expand_call_inline
>>>>
>>>> This is just a precautionary fix.
>>>>
>>>> 2021-01-04  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>>>
>>>> 	* tree-inline.c (expand_call_inline): Restore input_location.
>>>> 	Return result from recursive call.
>>> I suspect that we're always supposed to inline in this case.  As
>>> asserting that successfully_inlined is true before jumping to "egress"
>>> seems wise.
>>>
>>> OK with that change after the usual testing.
>>>
>> No this does not work:
>>
>> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++98 (internal compiler error)
>> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++98 (test for excess errors)
>> +UNRESOLVED: g++.dg/ipa/devirt-5.C  -std=gnu++98 compilation failed to produce executable
>> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++14 (internal compiler error)
>> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++14 (test for excess errors)
>> +UNRESOLVED: g++.dg/ipa/devirt-5.C  -std=gnu++14 compilation failed to produce executable
>> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++17 (internal compiler error)
>> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++17 (test for excess errors)
>> +UNRESOLVED: g++.dg/ipa/devirt-5.C  -std=gnu++17 compilation failed to produce executable
>> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++2a (internal compiler error)
>> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++2a (test for excess errors)
>> +UNRESOLVED: g++.dg/ipa/devirt-5.C  -std=gnu++2a compilation failed to produce executable
>> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++98 (internal compiler error)
>> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++98 (test for excess errors)
>> +UNRESOLVED: g++.dg/ipa/devirt-c-4.C  -std=gnu++98 compilation failed to produce executable
>> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++14 (internal compiler error)
>> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++14 (test for excess errors)
>> +UNRESOLVED: g++.dg/ipa/devirt-c-4.C  -std=gnu++14 compilation failed to produce executable
>> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++17 (internal compiler error)
>> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++17 (test for excess errors)
>> +UNRESOLVED: g++.dg/ipa/devirt-c-4.C  -std=gnu++17 compilation failed to produce executable
>> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++2a (internal compiler error)
>> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++2a (test for excess errors)
>> +UNRESOLVED: g++.dg/ipa/devirt-c-4.C  -std=gnu++2a compilation failed to produce executable
>> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++98 (internal compiler error)
>> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++98 (test for excess errors)
>> +UNRESOLVED: g++.dg/ipa/imm-devirt-2.C  -std=gnu++98 compilation failed to produce executable
>> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++14 (internal compiler error)
>> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++14 (test for excess errors)
>> +UNRESOLVED: g++.dg/ipa/imm-devirt-2.C  -std=gnu++14 compilation failed to produce executable
>> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++17 (internal compiler error)
>> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++17 (test for excess errors)
>> +UNRESOLVED: g++.dg/ipa/imm-devirt-2.C  -std=gnu++17 compilation failed to produce executable
>> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++2a (internal compiler error)
>> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++2a (test for excess errors)
>> +UNRESOLVED: g++.dg/ipa/imm-devirt-2.C  -std=gnu++2a compilation failed to produce executable
>> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++98 (internal compiler error)
>> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++98 (test for excess errors)
>> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++14 (internal compiler error)
>> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++14 (test for excess errors)
>> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++17 (internal compiler error)
>> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++17 (test for excess errors)
>> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++2a (internal compiler error)
>> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++2a (test for excess errors)
>> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++98 (internal compiler error)
>> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++98 (test for excess errors)
>> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++14 (internal compiler error)
>> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++14 (test for excess errors)
>> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++17 (internal compiler error)
>> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++17 (test for excess errors)
>> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++2a (internal compiler error)
>> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++2a (test for excess errors)
>> +FAIL: g++.dg/ipa/pr85421.C   (internal compiler error)
>> +FAIL: g++.dg/ipa/pr85421.C   (test for excess errors)
>> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++98 (internal compiler error)
>> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++98 (test for excess errors)
>> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++14 (internal compiler error)
>> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++14 (test for excess errors)
>> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++17 (internal compiler error)
>> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++17 (test for excess errors)
>> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++2a (internal compiler error)
>> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++2a (test for excess errors)
>> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++98 (internal compiler error)
>> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++98 (test for excess errors)
>> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++14 (internal compiler error)
>> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++14 (test for excess errors)
>> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++17 (internal compiler error)
>> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++17 (test for excess errors)
>> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++2a (internal compiler error)
>> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++2a (test for excess errors)
>>  FAIL: g++.dg/guality/pr55665.C   -O2 -flto -fno-use-linker-plugin -flto-partition=none  line 23 p == 40
>> +FAIL: g++.dg/lto/devirt-5 cp_lto_devirt-5_0.o-cp_lto_devirt-5_0.o link, -O3 -fno-early-inlining -fno-inline -fdump-ipa-cp -fdump-tree-optimized -flto (internal compiler error)
>>
>> Is it OK in the original form?
> Can you add a comment like
>
>   /* This used to return true even though we do fail to inline in
>      some cases.  See PRXYZ.  */
>
> and file a bug?
>
> OK with that change.
But what about the failures above.  What's effectively going on is we're
assuming inlining was successful and Bernd's patch shows that there's
cases where it isn't.  That probably needs deeper investigation.

>
> Some RAII-style auto_input_location class might be useful -
> temporarily setting input_location for some "legacy" code is
> the only valid use of input_location in the middle-end.
Agreed.

jeff
Bernd Edlinger Jan. 5, 2021, 5:36 p.m. UTC | #5 
On 1/5/21 5:51 PM, Jeff Law wrote:
> 
> 
> On 1/5/21 1:05 AM, Richard Biener wrote:
>> On Tue, 5 Jan 2021, Bernd Edlinger wrote:
>>
>>>
>>> On 1/4/21 10:23 PM, Jeff Law wrote:
>>>>
>>>> On 1/4/21 1:12 PM, Bernd Edlinger wrote:
>>>>> Hi,
>>>>>
>>>>> I spotted a place where input_location is clobbered accidentally.
>>>>>
>>>>> That is in a recursive call to expand_call_inline.  The input_location
>>>>> is usually restored by goto egress in this function.
>>>>>
>>>>> Additionally the return value of the recursive expand call is thrown
>>>>> away, which does not look like a good idea.
>>>>>
>>>>> Although this causes no problems ATM, I wanted to fix it anyway.
>>>>>
>>>>>
>>>>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>>>>> Is it OK for trunk?
>>>>>
>>>>>
>>>>> Thanks
>>>>> Bernd.
>>>>>
>>>>> 0001-Restore-input_location-after-recursive-expand_call_i.patch
>>>>>
>>>>> From 88b963bba7b32972abf0ea44a01c03d643d7c6ca Mon Sep 17 00:00:00 2001
>>>>> From: Bernd Edlinger <bernd.edlinger@hotmail.de>
>>>>> Date: Mon, 4 Jan 2021 11:35:31 +0100
>>>>> Subject: [PATCH] Restore input_location after recursive expand_call_inline
>>>>>
>>>>> This is just a precautionary fix.
>>>>>
>>>>> 2021-01-04  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>>>>
>>>>> 	* tree-inline.c (expand_call_inline): Restore input_location.
>>>>> 	Return result from recursive call.
>>>> I suspect that we're always supposed to inline in this case.  As
>>>> asserting that successfully_inlined is true before jumping to "egress"
>>>> seems wise.
>>>>
>>>> OK with that change after the usual testing.
>>>>
>>> No this does not work:
>>>
>>> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++98 (internal compiler error)
>>> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++98 (test for excess errors)
>>> +UNRESOLVED: g++.dg/ipa/devirt-5.C  -std=gnu++98 compilation failed to produce executable
>>> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++14 (internal compiler error)
>>> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++14 (test for excess errors)
>>> +UNRESOLVED: g++.dg/ipa/devirt-5.C  -std=gnu++14 compilation failed to produce executable
>>> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++17 (internal compiler error)
>>> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++17 (test for excess errors)
>>> +UNRESOLVED: g++.dg/ipa/devirt-5.C  -std=gnu++17 compilation failed to produce executable
>>> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++2a (internal compiler error)
>>> +FAIL: g++.dg/ipa/devirt-5.C  -std=gnu++2a (test for excess errors)
>>> +UNRESOLVED: g++.dg/ipa/devirt-5.C  -std=gnu++2a compilation failed to produce executable
>>> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++98 (internal compiler error)
>>> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++98 (test for excess errors)
>>> +UNRESOLVED: g++.dg/ipa/devirt-c-4.C  -std=gnu++98 compilation failed to produce executable
>>> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++14 (internal compiler error)
>>> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++14 (test for excess errors)
>>> +UNRESOLVED: g++.dg/ipa/devirt-c-4.C  -std=gnu++14 compilation failed to produce executable
>>> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++17 (internal compiler error)
>>> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++17 (test for excess errors)
>>> +UNRESOLVED: g++.dg/ipa/devirt-c-4.C  -std=gnu++17 compilation failed to produce executable
>>> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++2a (internal compiler error)
>>> +FAIL: g++.dg/ipa/devirt-c-4.C  -std=gnu++2a (test for excess errors)
>>> +UNRESOLVED: g++.dg/ipa/devirt-c-4.C  -std=gnu++2a compilation failed to produce executable
>>> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++98 (internal compiler error)
>>> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++98 (test for excess errors)
>>> +UNRESOLVED: g++.dg/ipa/imm-devirt-2.C  -std=gnu++98 compilation failed to produce executable
>>> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++14 (internal compiler error)
>>> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++14 (test for excess errors)
>>> +UNRESOLVED: g++.dg/ipa/imm-devirt-2.C  -std=gnu++14 compilation failed to produce executable
>>> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++17 (internal compiler error)
>>> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++17 (test for excess errors)
>>> +UNRESOLVED: g++.dg/ipa/imm-devirt-2.C  -std=gnu++17 compilation failed to produce executable
>>> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++2a (internal compiler error)
>>> +FAIL: g++.dg/ipa/imm-devirt-2.C  -std=gnu++2a (test for excess errors)
>>> +UNRESOLVED: g++.dg/ipa/imm-devirt-2.C  -std=gnu++2a compilation failed to produce executable
>>> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++98 (internal compiler error)
>>> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++98 (test for excess errors)
>>> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++14 (internal compiler error)
>>> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++14 (test for excess errors)
>>> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++17 (internal compiler error)
>>> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++17 (test for excess errors)
>>> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++2a (internal compiler error)
>>> +FAIL: g++.dg/ipa/pr71146.C  -std=gnu++2a (test for excess errors)
>>> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++98 (internal compiler error)
>>> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++98 (test for excess errors)
>>> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++14 (internal compiler error)
>>> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++14 (test for excess errors)
>>> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++17 (internal compiler error)
>>> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++17 (test for excess errors)
>>> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++2a (internal compiler error)
>>> +FAIL: g++.dg/ipa/pr79776.C  -std=gnu++2a (test for excess errors)
>>> +FAIL: g++.dg/ipa/pr85421.C   (internal compiler error)
>>> +FAIL: g++.dg/ipa/pr85421.C   (test for excess errors)
>>> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++98 (internal compiler error)
>>> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++98 (test for excess errors)
>>> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++14 (internal compiler error)
>>> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++14 (test for excess errors)
>>> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++17 (internal compiler error)
>>> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++17 (test for excess errors)
>>> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++2a (internal compiler error)
>>> +FAIL: g++.dg/ipa/pr91969.C  -std=gnu++2a (test for excess errors)
>>> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++98 (internal compiler error)
>>> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++98 (test for excess errors)
>>> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++14 (internal compiler error)
>>> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++14 (test for excess errors)
>>> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++17 (internal compiler error)
>>> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++17 (test for excess errors)
>>> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++2a (internal compiler error)
>>> +FAIL: g++.dg/ipa/pr92454.C  -std=gnu++2a (test for excess errors)
>>>  FAIL: g++.dg/guality/pr55665.C   -O2 -flto -fno-use-linker-plugin -flto-partition=none  line 23 p == 40
>>> +FAIL: g++.dg/lto/devirt-5 cp_lto_devirt-5_0.o-cp_lto_devirt-5_0.o link, -O3 -fno-early-inlining -fno-inline -fdump-ipa-cp -fdump-tree-optimized -flto (internal compiler error)
>>>
>>> Is it OK in the original form?
>> Can you add a comment like
>>
>>   /* This used to return true even though we do fail to inline in
>>      some cases.  See PRXYZ.  */
>>
>> and file a bug?
>>
>> OK with that change.
> But what about the failures above.  What's effectively going on is we're
> assuming inlining was successful and Bernd's patch shows that there's
> cases where it isn't.  That probably needs deeper investigation.
> 

Yes.  I opened https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98525
for that.


Bernd.

>>
>> Some RAII-style auto_input_location class might be useful -
>> temporarily setting input_location for some "legacy" code is
>> the only valid use of input_location in the middle-end.
> Agreed.
> 
> jeff
>
diff mbox series

Patch

From 88b963bba7b32972abf0ea44a01c03d643d7c6ca Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Mon, 4 Jan 2021 11:35:31 +0100
Subject: [PATCH] Restore input_location after recursive expand_call_inline

This is just a precautionary fix.

2021-01-04  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* tree-inline.c (expand_call_inline): Restore input_location.
	Return result from recursive call.
---
 gcc/tree-inline.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 360b85f..9f7d914 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -4840,9 +4840,9 @@  expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id,
       gimple_call_set_fndecl (stmt, edge->callee->decl);
       update_stmt (stmt);
       id->src_node->remove ();
-      expand_call_inline (bb, stmt, id, to_purge);
+      successfully_inlined = expand_call_inline (bb, stmt, id, to_purge);
       maybe_remove_unused_call_args (cfun, stmt);
-      return true;
+      goto egress;
     }
   fn = cg_edge->callee->decl;
   cg_edge->callee->get_untransformed_body ();
-- 
1.9.1