Hi Segher,
On Thu, Oct 01, 2020 at 01:22:07PM -0500, Segher Boessenkool wrote:
> Hi!
>
> On Thu, Oct 01, 2020 at 10:57:48PM +0930, Alan Modra wrote:
> > On Wed, Sep 30, 2020 at 03:56:32PM -0500, Segher Boessenkool wrote:
> > > On Wed, Sep 30, 2020 at 05:01:45PM +0930, Alan Modra wrote:
> > > > * config/rs6000/linux64.h (SUBSUBTARGET_OVERRIDE_OPTIONS): Don't
> > > > set -mcmodel=small for -mno-minimal-toc when pcrel.
> > >
> > > > - SET_CMODEL (CMODEL_SMALL); \
> > > > + if (TARGET_MINIMAL_TOC \
> > > > + || !(TARGET_PCREL \
> > > > + || (PCREL_SUPPORTED_BY_OS \
> > > > + && (rs6000_isa_flags_explicit \
> > > > + & OPTION_MASK_PCREL) == 0))) \
> > > > + SET_CMODEL (CMODEL_SMALL); \
> > >
> > > Please write this in a more readable way? With some "else" statements,
> > > perhaps.
> > >
> > > It is also fine to SET_CMODEL twice if that makes for simpler code.
> >
> > Committed as per your suggestion.
>
> Thanks.
>
> > I was looking at it again today
> > with the aim of converting this ugly macro to a function, and spotted
> > the duplication in freebsd64.h. Which has some bit-rot.
> >
> > Do you like the following? rs6000_linux64_override_options is
> > functionally the same as what was in linux64.h. I built various
> > configurations to test the change, powerpc64-linux, powerpc64le-linux
> > without any 32-bit targets enabled, powerpc64-freebsd12.0.
>
> Please do this as two patches? One the refactoring without any
> functional changes (which is pre-approved -- the name "linux64" isn't
> great if you use it in other OSes as well, but I cannot think of a
> better name either),
The patch as posted has no functional changes. I even avoided
formatting changes as much as possible. The only changes were those
necessary to use the code from linux64.h in a powerpc64-freebsd
compiler, where
#define TARGET_PROFILE_KERNEL 0
..
TARGET_PROFILE_KERNEL = 0;
doesn't work, nor does
if (!RS6000_BI_ARCH_P)
error (INVALID_32BIT, "32");
when RS6000_BI_ARCH_P is undefined.
> and the other the actual change (which probably is
> fine as well, but it is hard to see with the patch like this).
I do have a followup patch.. Commit c6be439b37 wrongly left a block
of code inside and "else" block, which changed the default for power10
TARGET_NO_FP_IN_TOC accidentally. We don't want FP constants in the
TOC when -mcmodel=medium can address them just as efficiently outside
the TOC.
* config/rs6000/rs6000.c (rs6000_linux64_override_options):
Formatting. Correct setting of TARGET_NO_FP_IN_TOC and
TARGET_NO_SUM_IN_TOC.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 48f3cdec440..a1651551ff2 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -3493,8 +3493,7 @@ rs6000_linux64_override_options ()
}
if (!global_options_set.x_rs6000_current_cmodel)
SET_CMODEL (CMODEL_MEDIUM);
- if ((rs6000_isa_flags_explicit
- & OPTION_MASK_MINIMAL_TOC) != 0)
+ if ((rs6000_isa_flags_explicit & OPTION_MASK_MINIMAL_TOC) != 0)
{
if (global_options_set.x_rs6000_current_cmodel
&& rs6000_current_cmodel != CMODEL_SMALL)
@@ -3503,23 +3502,18 @@ rs6000_linux64_override_options ()
SET_CMODEL (CMODEL_SMALL);
else if (TARGET_PCREL
|| (PCREL_SUPPORTED_BY_OS
- && (rs6000_isa_flags_explicit
- & OPTION_MASK_PCREL) == 0))
+ && (rs6000_isa_flags_explicit & OPTION_MASK_PCREL) == 0))
/* Ignore -mno-minimal-toc. */
;
else
SET_CMODEL (CMODEL_SMALL);
}
- else
+ if (rs6000_current_cmodel != CMODEL_SMALL)
{
- if (rs6000_current_cmodel != CMODEL_SMALL)
- {
- if (!global_options_set.x_TARGET_NO_FP_IN_TOC)
- TARGET_NO_FP_IN_TOC
- = rs6000_current_cmodel == CMODEL_MEDIUM;
- if (!global_options_set.x_TARGET_NO_SUM_IN_TOC)
- TARGET_NO_SUM_IN_TOC = 0;
- }
+ if (!global_options_set.x_TARGET_NO_FP_IN_TOC)
+ TARGET_NO_FP_IN_TOC = rs6000_current_cmodel == CMODEL_MEDIUM;
+ if (!global_options_set.x_TARGET_NO_SUM_IN_TOC)
+ TARGET_NO_SUM_IN_TOC = 0;
}
if (TARGET_PLTSEQ && DEFAULT_ABI != ABI_ELFv2)
{
--
Alan Modra
Australia Development Lab, IBM
