Message ID | AM0PR0602MB3410D0C853878A494D56E5D5E4D20@AM0PR0602MB3410.eurprd06.prod.outlook.com |
---|---|
State | New |
Headers | show |
Series | Restore input_location after recursive expand_call_inline | expand |
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
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 >
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 > > >
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
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 >
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