On Tue, Aug 27, 2013 at 04:34:09PM +0100, Janis Johnson wrote:
> On 08/27/2013 06:52 AM, Marcus Shawcroft wrote:
> > On 23 July 2013 17:40, Janis Johnson <janis_john...@mentor.com> wrote:
> >> On 07/22/2013 02:59 AM, Vidya Praveen wrote:
> >>> Hello,
> >>>
> >>> There are 42 test files (25 under gcc.dg) that specifies
> >>>
> >>> { dg-add-options bind_pic_locally }
> >>>
> >>> in the regression testsuite. The procedure
> >>> add_options_for_bind_pic_locally
> >>> from lib/target-supports.exp adds -fPIE or -fpie when -fPIC or -fpic is
> >>> passed
> >>> respectively. But this is added before the dejagnu multilib options are
> >>> added.
> >>> So when -fPIC is passed as a multilib option, -fPIE will be unset by -fPIC
> >>> (see gcc/common.opt). This should have been the behaviour since the patch
> >>> http://gcc.gnu.org/ml/gcc-patches/2012-11/msg01026.html that brings all
> >>> -fPIC
> >>> & -fPIE variants in a Negative loop, was applied.
> >>>
> >>> I tried fixing this in dejagnu/target.exp by adding the multilib options
> >>> before
> >>> the other options:
> >>>
> >>> default_target_compile:
> >>>
> >>> < append add_flags " [board_info $dest multilib_flags]"
> >>> ---
> >>>> set add_flags " [board_info $dest multilib_flags] $add_flags"
> >>>
> >>> and ran regressions for x86_64-unknown-linux-gnu before and after the
> >>> change.
> >>> The only difference in the results after the change was 24 new PASSes
> >>> which
> >>> are from the testcases which either use bind_pic_locally or that use
> >>> -fno-pic.
> >>>
> >>> (Interestingly, there are many test files that bind_pic_locally pass
> >>> without
> >>> any issue before and after the change.)
> >>>
> >>> I tend to think that the options added from the test files should always
> >>> win.
> >>> Please correct me if I'm wrong. If I'm right, is dejagnu/target.exp is the
> >>> best place to fix this and the way it tried to fix? Any better
> >>> suggestions?
> >>>
> >>> Though this case is to do with -fPIC, I'm sure there are other options
> >>> which
> >>> when they come as multilib options might have same issue with the some of
> >>> the
> >>> options added by the test files or the default options.
> >>>
> >>> Regards
> >>> VP
> >>
> >> Ideally we would ask for that change in DejaGnu, but relying on such a
> >> change would require everyone testing GCC to upgrade to a version of
> >> DejaGnu with that fix, and I don't think we're ready to do that.
> >>
> >> Tests that add options that might override or conflict with multilib
> >> flags should check for those flags first and skip the test if they are
> >> used. For examples, see tests in gcc.target/arm that use dg-skip-if.
> >
> > Umm, the purpose of bind_pic_locally appears to be to detect the use
> > of -fPIC and override that behavior with -fPIE. If I understand the
> > above paragraph correctly then bind_pic_locally is fundamentally
> > broken and all of the tests that use it need rewriting to skip if
> > -fPIC or -fpic is in use?
> >
> > Cheers
> > /Marcus
> >
>
> That is correct. There should probably be an effective-target check
> bind_pic_locally_ok that fails if adding -fpie or -fPIE doesn't have the
> expected result, and the tests that use "dg-add-options bind_pic_locally"
> should be skipped if bind_pic_locally_ok fails.
>
> Janis
>
Janis, whether we pass -fPIC/-fpic through multilib_flags or through cflags
bind_pic_locally remains to be broken. So I am wondering if it's really
necessary to go through bind_pic_locally_ok route. Instead could we just
replace all the uses of bind_pic_locally with dg-skip-if and perhaps remove
the definition of bind_pic_locally to avoid future use of it?
Cheers
VP
