Re: [PATCH] Restore input_location after recursive expand_call_inline

[Jeff Law <law@redhat.com>]

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