Re: [PATCH, V3, #5 of 10], Make -mpcrel default on little endian Linux systems

[Segher Boessenkool]

On Tue, Sep 03, 2019 at 05:06:52PM -0400, Michael Meissner wrote:
> On Fri, Aug 30, 2019 at 01:32:57PM -0500, Segher Boessenkool wrote:
> > On Mon, Aug 26, 2019 at 05:07:25PM -0400, Michael Meissner wrote:
> > > +/* By default enable support for pc-relative and numeric prefixed 
> > > addressing on
> > > +   the 'future' system, unless it is overriden at build time.  */
> > > +#ifndef TARGET_PREFIXED_ADDR_DEFAULT
> > > +#define TARGET_PREFIXED_ADDR_DEFAULT     1
> > > +#endif
> > > +
> > > +#if !defined (TARGET_PCREL_DEFAULT) && TARGET_PREFIXED_ADDR_DEFAULT
> > > +#define TARGET_PCREL_DEFAULT             1
> > > +#endif
> > 
> > Spelling ("overridden").
> > 
> > How can it be overridden at build time?
> > 
> > How can it be defined already, when linux64.h is included?  Don't put in
> > guards against things that cannot happen.
> 
> You can define TARGET_PREFIXED_ADDR_DEFAULT or TARGET_PCREL_DEFAULT in your
> CFLAGS or via the make command line (which is how I tested it).

Rebuilding GCC?  Yeah, that doesn't count, sorry :-)

It's fine to put some sanity check asserts in somewhere, if you are
worried about such things.  But please don't add unnecessary conditions
to macros like this.  If something unexpected happened, we want to hear
about it (either directly, e.g. via an assert, or indirectly, via crash
and burn), it should not be hidden.

> > > +      /* Enable defaults if desired.  */
> > > +      else
> > > + {
> > > +   if (!explicit_prefixed
> > > +       && (TARGET_PREFIXED_ADDR_DEFAULT
> > > +           || TARGET_PCREL
> > > +           || TARGET_PCREL_DEFAULT))
> > > +     rs6000_isa_flags |= OPTION_MASK_PREFIXED_ADDR;
> > > +
> > > +   if (!explicit_pcrel && TARGET_PCREL_DEFAULT
> > > +       && TARGET_CMODEL == CMODEL_MEDIUM)
> > > +     rs6000_isa_flags |= OPTION_MASK_PCREL;
> > > + }
> > 
> > Should these be the other way around?
> 
> I'm not sure I follow the question.  You want to enable pc-relative support if
> prefixed addressing support is enabled, and the OS says that it supports
> pc-relative addressing.
> 
> If you previously disabled prefixed addressing, you can't enable pc-relative 
> by default.

I meant, should it instead be:

          if (!explicit_pcrel && TARGET_PCREL_DEFAULT
              && TARGET_CMODEL == CMODEL_MEDIUM)
            rs6000_isa_flags |= OPTION_MASK_PCREL;
        }

          if (!explicit_prefixed
              && (TARGET_PREFIXED_ADDR_DEFAULT
                  || TARGET_PCREL
                  || TARGET_PCREL_DEFAULT))
            rs6000_isa_flags |= OPTION_MASK_PREFIXED_ADDR;

i.e., if we enable PCREL, shouldn't we also enable PREFIXED_ADDR?


Segher