Re: [PATCH 2/2] [ARM] Add support for -mpure-code in thumb-1 (v6m)

[Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>]

Hi Christophe,

On 12/17/19 3:31 PM, Kyrill Tkachov wrote:
> On 12/17/19 2:33 PM, Christophe Lyon wrote:
> > On Tue, 17 Dec 2019 at 11:34, Kyrill Tkachov
> > <kyrylo.tkachov@foss.arm.com> wrote:
> > > Hi Christophe,
> > >
> > > On 11/18/19 9:00 AM, Christophe Lyon wrote:
> > > > On Wed, 13 Nov 2019 at 15:46, Christophe Lyon
> > > > <christophe.lyon@linaro.org> wrote:
> > > > > On Tue, 12 Nov 2019 at 12:13, Richard Earnshaw (lists)
> > > > > <Richard.Earnshaw@arm.com> wrote:
> > > > > > On 18/10/2019 14:18, Christophe Lyon wrote:
> > > > > > > +      bool not_supported = arm_arch_notm || flag_pic ||
> > > > TARGET_NEON;
> > > > > > This is a poor name in the context of the function as a whole.  
> > > > > > What's
> > > > > > not supported.  Please think of a better name so that I have some 
> > > > > > idea
> > > > > > what the intention is.
> > > > > That's to keep most of the code common when checking if -mpure-code
> > > > > and -mslow-flash-data are supported.
> > > > > These 3 cases are common to the two compilation flags, and
> > > > > -mslow-flash-data still needs to check TARGET_HAVE_MOVT in addition.
> > > > >
> > > > > Would "common_unsupported_modes" work better for you?
> > > > > Or I can duplicate the "arm_arch_notm || flag_pic || TARGET_NEON" in
> > > > > the two tests.
> > > > Hi,
> > > >
> > > > Here is an updated version, using "common_unsupported_modes" instead
> > > > of "not_supported", and fixing the typo reported by Kyrill.
> > > > The ChangeLog is still the same.
> > > >
> > > > OK?
> > >
> > > The name looks ok to me. Richard had a concern about Armv8-M Baseline,
> > > but I do see it being supported as you pointed out.
> > >
> > > So I believe all the concerns are addressed.
> > OK, thanks!
> >
> > > Thus the code is ok. However, please also updated the documentation for
> > > -mpure-code in invoke.texi (it currently states that a MOVT instruction
> > > is needed).
> > I didn't think about this :(
> > It currently says: "This option is only available when generating
> > non-pic code for M-profile targets with the MOVT instruction."
> >
> > I suggest to remove the "with the MOVT instruction" part. Is that OK
> > if I commit my patch and this doc change?
>
> Yes, I think that is simplest correct change to make.

Can you also send a patch to the changes.html page for GCC 10 making 
users aware that this restriction is now lifted?

Thanks,

Kyrill


> Thanks,
>
> Kyrill
>
>
> > Christophe
> >
> > > Thanks,
> > >
> > > Kyrill
> > >
> > >
> > >
> > > > Thanks,
> > > >
> > > > Christophe
> > > >
> > > > > Thanks,
> > > > >
> > > > > Christophe
> > > > >
> > > > > > R.