Re: [PATCH] Restore input_location after recursive expand_call_inline

[Bernd Edlinger <bernd.edlinger@hotmail.de>]

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
>