On 13 January 2017 at 11:22, Renlin Li <renlin...@foss.arm.com> wrote:
> Hi Christophe,
>
> Thanks for testing the patch!
> I check the test case gcc.dg/lto/pr54709, it seems the test case is not
> properly written.
>
> It add extra ld option -shared without checking the target support for that.
> After the change, this compilation will fail as a regression.
> IIUC, '-shared' option is required, it should be gated with corresponding
> target selector.
>
> "g++.dg/ipa/devirt-28a.C" now is skipped because of the target selector
> there.
> // { dg-do link { target { { gld && fpic } && shared } } }
>
> perhaps "gcc.dg/lto/pr54709" should do similar things like this:
> // { dg-do link { target { shared } } }
Quite likely, indeed.> > > As far as I know, with different cpu/arch configurations, different > relocations are generated in the library, some of the relocations are not > allowed to be used in shared > object. > > With -march=armv7-a (and the --with-cpu=cortex-a9 option you mentioned), the > linking stage of the test will fail because of this error: > "relocation XXX against external symbol `YYY' can not be used when making a > shared object" > for instance: crtbegin.o: relocation R_ARM_MOVW_ABS_NC against `a local > symbol` can not be used when making a shared object; recompile with -fPIC > > If you are luck enough, for example with arm7tdmi cpu, no such relocation is > generated in startup files. The "shared" target support check will pass for > simple and naive code. > "--with-cpu=cortex-m3" should be this case. But the test cases which require > shared object support will fail. > > > So this "shared" target checking mechanism is not reliable. The patch is to > change this. > Shouldn't your patch imply that several tests move from "fail" to "unsupported" on armv7-a ? I'm surprised not to see any difference in the results. > > > Regards, > Renlin > > > > > On 13/01/17 08:48, Christophe Lyon wrote: >> >> Hi Renlin, >> >> >> On 12 January 2017 at 16:50, Renlin Li <renlin...@foss.arm.com> wrote: >>> >>> Hi Kugan, >>> >>> some of the targets do include pie, and use the same crtbegin file as >>> shared >>> object. >>> For example, alpha/elf.h >>> >>> And there are targets which don't do that, >>> For example, sh/elf.h >>> >>> Most of the elf target seem only consider the simple case. >>> >>> The purpose of this patch is to make it possible to correctly check >>> whether >>> current toolchain supports shared object. >>> >>> Current dejegnu target selector "shared" tries to compile a simple source >>> code to with "-shared -fpic" options to check whether "-shared" is >>> supported. >>> >>> For arm baremetal targets with, this is not sufficient. >>> >>> arm-none-eabi is built with multilib. When running this testcase, if it's >>> compiled with "-march=armv7-a". >>> The crtbegin.o for this architecture version contains relocations which >>> cannot be used in shared object. >>> This test will fail. >>> >>> if no cpu or architecture is specified, the default cpu will be arm7tdmi. >>> The test will pass as crtbegin.o for this version doesn't contains any >>> relocations >>> only allowed in shared object. >>> >>> To make this "shared" target selector work for arm baremetal toolchain. >>> The >>> correct way is to use different startup file for shared and non-shared >>> toolchain. >>> >>> If the toolchain doesn't support "-shared" option, and someone attempts >>> to >>> use it >>> to create shared object, it will complaint that "crtbeginS.o" cannot not >>> be >>> found. >>> >> >> I have run validations with your patch, and noticed regressions on >> arm-none-eabi >> when using default cpu or --with-cpu=cortex-m3: >> >> - PASS now FAIL [PASS => FAIL]: >> >> gcc.dg/lto/pr54709 c_lto_pr54709_0.o-c_lto_pr54709_1.o link, -fPIC >> -fvisibility=hidden -flto >> gcc.dg/lto/pr61526 c_lto_pr61526_0.o-c_lto_pr61526_1.o link, -fPIC >> -flto -flto-partition=1to1 >> gcc.dg/lto/pr64415 c_lto_pr64415_0.o-c_lto_pr64415_1.o link, -O -flto >> -fpic >> >> on the same configurations, I've noticed these improvements: >> >> g++.dg/ipa/devirt-28a.C -std=gnu++11 (test for excess errors) >> g++.dg/ipa/devirt-28a.C -std=gnu++14 (test for excess errors) >> g++.dg/ipa/devirt-28a.C -std=gnu++98 (test for excess errors) >> are now unsupported rather than fail. >> >> Why is it different when the toolchain is configured --with-cpu=cortex-a9 >> for instance? Are the tests involving -shared already skipped in this >> case? >> >>> A full history of discussion is here: >>> https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00322.html >>> >>> >>> Regards, >>> Renlin >>> >>> >>> >>> On 12/01/17 11:47, kugan wrote: >>>> >>>> >>>> Hi, >>>> >>>> On 16/06/16 21:04, Renlin Li wrote: >>>>> >>>>> >>>>> /* Now we define the strings used to build the spec file. */ >>>>> -#define UNKNOWN_ELF_STARTFILE_SPEC " crti%O%s crtbegin%O%s >>>>> crt0%O%s" >>>>> +#define UNKNOWN_ELF_STARTFILE_SPEC \ >>>>> + "crti%O%s \ >>>>> + %{!shared:crtbegin%O%s} %{shared:crtbeginS%O%s} \ >>>>> + crt0%O%s" >>>> >>>> >>>> >>>> Some targets seems to use shared|pie. When you change it, shouldn't it >>>> also include for pie? >>>> >>>> Thanks, >>>> Kugan
