On Tue, 17 Dec 2019 at 11:34, Kyrill Tkachov <kyrylo.tkac...@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.l...@linaro.org> wrote: > > > > > > On Tue, 12 Nov 2019 at 12:13, Richard Earnshaw (lists) > > > <richard.earns...@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? Christophe > Thanks, > > Kyrill > > > > > > > Thanks, > > > > Christophe > > > > > Thanks, > > > > > > Christophe > > > > > > > > > > > R.
