[GCC/ARM] Fix PR87374: ICE with -mslow-flash-data and -mword-relocations

Message ID CAKnkMGtgViqG9J3kF_MtKmOU9r2rfLrcu_Vw8GE8n71QbROVtQ@mail.gmail.com
State New
Headers show
Series
  • [GCC/ARM] Fix PR87374: ICE with -mslow-flash-data and -mword-relocations
Related show

Commit Message

Thomas Preudhomme Sept. 26, 2018, 5:39 p.m.
Hi,

GCC ICEs under -mslow-flash-data and -mword-relocations because there
is no way to load an address, both literal pools and MOVW/MOVT being
forbidden. This patch gives an error message when both options are
specified by the user and adds the according dg-skip-if directives for
tests that use either of these options.

ChangeLog entries are as follows:

*** gcc/ChangeLog ***

2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

    PR target/87374
    * config/arm/arm.c (arm_option_check_internal): Disable the combined
    use of -mslow-flash-data and -mword-relocations.

*** gcc/testsuite/ChangeLog ***

2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

    PR target/87374
    * gcc.target/arm/movdi_movt.c: Skip if both -mslow-flash-data and
    -mword-relocations would be passed when compiling the test.
    * gcc.target/arm/movsi_movt.c: Likewise.
    * gcc.target/arm/pr81863.c: Likewise.
    * gcc.target/arm/thumb2-slow-flash-data-1.c: Likewise.
    * gcc.target/arm/thumb2-slow-flash-data-2.c: Likewise.
    * gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.
    * gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.
    * gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.
    * gcc.target/arm/tls-disable-literal-pool.c: Likewise.


Testing: Bootstrapped in Thumb-2 mode. No testsuite regression when
targeting arm-none-eabi. Modified tests get skipped as expected when
running the testsuite with -mslow-flash-data (pr81863.c) or
-mword-relocations (all the others).


Is this ok for trunk? I'd also appreciate guidance on whether this is
worth a backport. It's a simple patch but on the other hand it only
prevents some option combination, it does not fix anything so I have
mixed feelings.

Best regards,

Thomas

Comments

Kyrill Tkachov Sept. 27, 2018, 8:26 a.m. | #1
Hi Thomas,

On 26/09/18 18:39, Thomas Preudhomme wrote:
> Hi,

>

> GCC ICEs under -mslow-flash-data and -mword-relocations because there

> is no way to load an address, both literal pools and MOVW/MOVT being

> forbidden. This patch gives an error message when both options are

> specified by the user and adds the according dg-skip-if directives for

> tests that use either of these options.

>

> ChangeLog entries are as follows:

>

> *** gcc/ChangeLog ***

>

> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

>

>      PR target/87374

>      * config/arm/arm.c (arm_option_check_internal): Disable the combined

>      use of -mslow-flash-data and -mword-relocations.

>

> *** gcc/testsuite/ChangeLog ***

>

> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

>

>      PR target/87374

>      * gcc.target/arm/movdi_movt.c: Skip if both -mslow-flash-data and

>      -mword-relocations would be passed when compiling the test.

>      * gcc.target/arm/movsi_movt.c: Likewise.

>      * gcc.target/arm/pr81863.c: Likewise.

>      * gcc.target/arm/thumb2-slow-flash-data-1.c: Likewise.

>      * gcc.target/arm/thumb2-slow-flash-data-2.c: Likewise.

>      * gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.

>      * gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.

>      * gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.

>      * gcc.target/arm/tls-disable-literal-pool.c: Likewise.

>

>

> Testing: Bootstrapped in Thumb-2 mode. No testsuite regression when

> targeting arm-none-eabi. Modified tests get skipped as expected when

> running the testsuite with -mslow-flash-data (pr81863.c) or

> -mword-relocations (all the others).

>

>

> Is this ok for trunk? I'd also appreciate guidance on whether this is

> worth a backport. It's a simple patch but on the other hand it only

> prevents some option combination, it does not fix anything so I have

> mixed feelings.


In my opinion -mslow-flash-data is more of a tuning option rather than a security/ABI feature
and therefore erroring out on its combination with -mword-relocations feels odd.
I'm leaning more towards making -mword-relocations or any other option that really requires constant pools
to bypass/disable the effects of -mslow-flash-data instead.
For me, as a user, it would give a more friendly experience.
That said, you have more familiarity with M-profile users. Would such users prefer a hard error when the -mslow-flash-data option
has its effects bypassed? Maybe we could emit a warning rather than an error when this happens?

Thanks,
Kyrill

> Best regards,

>

> Thomas
Ramana Radhakrishnan Sept. 27, 2018, 10:14 a.m. | #2
On 27/09/2018 09:26, Kyrill Tkachov wrote:
> Hi Thomas,

> 

> On 26/09/18 18:39, Thomas Preudhomme wrote:

>> Hi,

>>

>> GCC ICEs under -mslow-flash-data and -mword-relocations because there

>> is no way to load an address, both literal pools and MOVW/MOVT being

>> forbidden. This patch gives an error message when both options are

>> specified by the user and adds the according dg-skip-if directives for

>> tests that use either of these options.

>>

>> ChangeLog entries are as follows:

>>

>> *** gcc/ChangeLog ***

>>

>> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

>>

>>       PR target/87374

>>       * config/arm/arm.c (arm_option_check_internal): Disable the combined

>>       use of -mslow-flash-data and -mword-relocations.

>>

>> *** gcc/testsuite/ChangeLog ***

>>

>> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

>>

>>       PR target/87374

>>       * gcc.target/arm/movdi_movt.c: Skip if both -mslow-flash-data and

>>       -mword-relocations would be passed when compiling the test.

>>       * gcc.target/arm/movsi_movt.c: Likewise.

>>       * gcc.target/arm/pr81863.c: Likewise.

>>       * gcc.target/arm/thumb2-slow-flash-data-1.c: Likewise.

>>       * gcc.target/arm/thumb2-slow-flash-data-2.c: Likewise.

>>       * gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.

>>       * gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.

>>       * gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.

>>       * gcc.target/arm/tls-disable-literal-pool.c: Likewise.

>>

>>

>> Testing: Bootstrapped in Thumb-2 mode. No testsuite regression when

>> targeting arm-none-eabi. Modified tests get skipped as expected when

>> running the testsuite with -mslow-flash-data (pr81863.c) or

>> -mword-relocations (all the others).

>>

>>

>> Is this ok for trunk? I'd also appreciate guidance on whether this is

>> worth a backport. It's a simple patch but on the other hand it only

>> prevents some option combination, it does not fix anything so I have

>> mixed feelings.

> 

> In my opinion -mslow-flash-data is more of a tuning option rather than a security/ABI feature

> and therefore erroring out on its combination with -mword-relocations feels odd.

> I'm leaning more towards making -mword-relocations or any other option that really requires constant pools

> to bypass/disable the effects of -mslow-flash-data instead.

> For me, as a user, it would give a more friendly experience.

> That said, you have more familiarity with M-profile users. Would such users prefer a hard error when the -mslow-flash-data option

> has its effects bypassed? Maybe we could emit a warning rather than an error when this happens?



-mslow-flash-data and -mword-relocations are contradictory in their 
expectations. mslow-flash-data is for not putting anything in the 
literal pool whereas mword-relocations is purely around the use of movw 
/ movt instructions for word sized values. I wish we had called 
-mslow-flash-data something else (probably -mno-literal-pools). 
-mslow-flash-data is used primarily by M-profile users and 
-mword-relocations IIUC was a point fix for use in the Linux kernel for 
module loads at a time when not all module loaders in the linux kernel 
were fixed for the movw / movt relocations and armv7-a / thumb2 was in 
it's infancy . Thus they are used by different constituencies in general 
and I wouldn't see them used together by actual users.

Considering the above, I would prefer a hard error rather than a warning 
as they are contradictory and I'd prefer that we error'd out. Further 
this bugzilla entry is probably created with fuzzing with a variety of 
options rather than from any real use case.

Oh and yes, lets update invoke.texi while here.


regards
Ramana



> 

> Thanks,

> Kyrill

> 

>> Best regards,

>>

>> Thomas

> 

>
Kyrill Tkachov Sept. 27, 2018, 10:16 a.m. | #3
On 27/09/18 11:13, Ramana Radhakrishnan wrote:
> On 27/09/2018 09:26, Kyrill Tkachov wrote:

>> Hi Thomas,

>>

>> On 26/09/18 18:39, Thomas Preudhomme wrote:

>>> Hi,

>>>

>>> GCC ICEs under -mslow-flash-data and -mword-relocations because there

>>> is no way to load an address, both literal pools and MOVW/MOVT being

>>> forbidden. This patch gives an error message when both options are

>>> specified by the user and adds the according dg-skip-if directives for

>>> tests that use either of these options.

>>>

>>> ChangeLog entries are as follows:

>>>

>>> *** gcc/ChangeLog ***

>>>

>>> 2018-09-25  Thomas Preud'homme <thomas.preudhomme@linaro.org>

>>>

>>>       PR target/87374

>>>       * config/arm/arm.c (arm_option_check_internal): Disable the combined

>>>       use of -mslow-flash-data and -mword-relocations.

>>>

>>> *** gcc/testsuite/ChangeLog ***

>>>

>>> 2018-09-25  Thomas Preud'homme <thomas.preudhomme@linaro.org>

>>>

>>>       PR target/87374

>>>       * gcc.target/arm/movdi_movt.c: Skip if both -mslow-flash-data and

>>>       -mword-relocations would be passed when compiling the test.

>>>       * gcc.target/arm/movsi_movt.c: Likewise.

>>>       * gcc.target/arm/pr81863.c: Likewise.

>>>       * gcc.target/arm/thumb2-slow-flash-data-1.c: Likewise.

>>>       * gcc.target/arm/thumb2-slow-flash-data-2.c: Likewise.

>>>       * gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.

>>>       * gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.

>>>       * gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.

>>>       * gcc.target/arm/tls-disable-literal-pool.c: Likewise.

>>>

>>>

>>> Testing: Bootstrapped in Thumb-2 mode. No testsuite regression when

>>> targeting arm-none-eabi. Modified tests get skipped as expected when

>>> running the testsuite with -mslow-flash-data (pr81863.c) or

>>> -mword-relocations (all the others).

>>>

>>>

>>> Is this ok for trunk? I'd also appreciate guidance on whether this is

>>> worth a backport. It's a simple patch but on the other hand it only

>>> prevents some option combination, it does not fix anything so I have

>>> mixed feelings.

>>

>> In my opinion -mslow-flash-data is more of a tuning option rather than a security/ABI feature

>> and therefore erroring out on its combination with -mword-relocations feels odd.

>> I'm leaning more towards making -mword-relocations or any other option that really requires constant pools

>> to bypass/disable the effects of -mslow-flash-data instead.

>

> -mslow-flash-data and -mword-relocations are contradictory in their expectations. mslow-flash-data is for not putting anything in the literal pool whereas mword-relocations is purely around the use of movw / movt instructions for word sized values. I wish we had called -mslow-flash-data something else (probably -mno-literal-pools). -mslow-flash-data is used primarily by M-profile users and -mword-relocations IIUC was a point fix for use in the Linux kernel for module loads at a time when not all module loaders in the linux kernel were fixed for the movw / movt relocations and armv7-a / thumb2 was in it's infancy :). Thus they are used by different constituencies in general and I wouldn't see them used together by actual users.

>


Ah, thank you for the background Ramana. The naming of mslow-flash-data is confusing indeed.
It sounds more like a tuning request rather than a hard requirement.

> Considering the above, I would prefer a hard error rather than a warning as they are contradictory and I'd prefer that we error'd out. Further this bugzilla entry is probably created with fuzzing with a variety of options rather than from any real use case.

>


Yes, in that case erroring out is easier.

> Oh and yes, lets update invoke.texi while here.

>


Yes. This deserves an entry in the documentation.

Thanks,
Kyrill

>

> regards

> Ramana

>

>

>> For me, as a user, it would give a more friendly experience.

>

>

>> That said, you have more familiarity with M-profile users. Would such users prefer a hard error when the -mslow-flash-data option

>> has its effects bypassed? Maybe we could emit a warning rather than an error when this happens?

>>

>> Thanks,

>> Kyrill

>>

>>> Best regards,

>>>

>>> Thomas

>>

>>

>
Thomas Preudhomme Oct. 2, 2018, 10:42 a.m. | #4
Hi Ramana,

On Thu, 27 Sep 2018 at 11:14, Ramana Radhakrishnan
<ramana.radhakrishnan@arm.com> wrote:
>

> On 27/09/2018 09:26, Kyrill Tkachov wrote:

> > Hi Thomas,

> >

> > On 26/09/18 18:39, Thomas Preudhomme wrote:

> >> Hi,

> >>

> >> GCC ICEs under -mslow-flash-data and -mword-relocations because there

> >> is no way to load an address, both literal pools and MOVW/MOVT being

> >> forbidden. This patch gives an error message when both options are

> >> specified by the user and adds the according dg-skip-if directives for

> >> tests that use either of these options.

> >>

> >> ChangeLog entries are as follows:

> >>

> >> *** gcc/ChangeLog ***

> >>

> >> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

> >>

> >>       PR target/87374

> >>       * config/arm/arm.c (arm_option_check_internal): Disable the combined

> >>       use of -mslow-flash-data and -mword-relocations.

> >>

> >> *** gcc/testsuite/ChangeLog ***

> >>

> >> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

> >>

> >>       PR target/87374

> >>       * gcc.target/arm/movdi_movt.c: Skip if both -mslow-flash-data and

> >>       -mword-relocations would be passed when compiling the test.

> >>       * gcc.target/arm/movsi_movt.c: Likewise.

> >>       * gcc.target/arm/pr81863.c: Likewise.

> >>       * gcc.target/arm/thumb2-slow-flash-data-1.c: Likewise.

> >>       * gcc.target/arm/thumb2-slow-flash-data-2.c: Likewise.

> >>       * gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.

> >>       * gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.

> >>       * gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.

> >>       * gcc.target/arm/tls-disable-literal-pool.c: Likewise.

> >>

> >>

> >> Testing: Bootstrapped in Thumb-2 mode. No testsuite regression when

> >> targeting arm-none-eabi. Modified tests get skipped as expected when

> >> running the testsuite with -mslow-flash-data (pr81863.c) or

> >> -mword-relocations (all the others).

> >>

> >>

> >> Is this ok for trunk? I'd also appreciate guidance on whether this is

> >> worth a backport. It's a simple patch but on the other hand it only

> >> prevents some option combination, it does not fix anything so I have

> >> mixed feelings.

> >

> > In my opinion -mslow-flash-data is more of a tuning option rather than a security/ABI feature

> > and therefore erroring out on its combination with -mword-relocations feels odd.

> > I'm leaning more towards making -mword-relocations or any other option that really requires constant pools

> > to bypass/disable the effects of -mslow-flash-data instead.

>

> -mslow-flash-data and -mword-relocations are contradictory in their

> expectations. mslow-flash-data is for not putting anything in the

> literal pool whereas mword-relocations is purely around the use of movw

> / movt instructions for word sized values. I wish we had called

> -mslow-flash-data something else (probably -mno-literal-pools).

> -mslow-flash-data is used primarily by M-profile users and

> -mword-relocations IIUC was a point fix for use in the Linux kernel for

> module loads at a time when not all module loaders in the linux kernel

> were fixed for the movw / movt relocations and armv7-a / thumb2 was in

> it's infancy :). Thus they are used by different constituencies in

> general and I wouldn't see them used together by actual users.


Technically, -mslow-flash-data does not forbid literal pool, it just
discourages it because it's slower than many instructions. -mpure-code
on the other hand reuse the same logic and does forbid literal pools.
We could treat -mslow-flash-data differently but the question is
whether it is worth the trouble.

By the way, I've noticed that the documentation for -mword-relocations
says it defaults to on for -fpic and -fPIC but when looking through
the code I saw that target_word_relocation is not set in these case,
rather the initial commit checks that introduced -mword-relocation
also checks for flag_pic when checking target_word_relocation. However
a later commit added one more check for target_word_relocations but
nothing for flag_pic. I'm now consolidating this so that flag_pic sets
target_word_relocations. I'll do a regression testing with -fPIC and
then post an updated patch.

>

> Considering the above, I would prefer a hard error rather than a warning

> as they are contradictory and I'd prefer that we error'd out. Further

> this bugzilla entry is probably created with fuzzing with a variety of

> options rather than from any real use case.

>

> Oh and yes, lets update invoke.texi while here.


Done. Will be part of the updated patch.

Best regards,

Thomas
Ramana Radhakrishnan Oct. 2, 2018, 12:39 p.m. | #5
On 02/10/2018 11:42, Thomas Preudhomme wrote:
> Hi Ramana,

> 

> On Thu, 27 Sep 2018 at 11:14, Ramana Radhakrishnan

> <ramana.radhakrishnan@arm.com> wrote:

>>

>> On 27/09/2018 09:26, Kyrill Tkachov wrote:

>>> Hi Thomas,

>>>

>>> On 26/09/18 18:39, Thomas Preudhomme wrote:

>>>> Hi,

>>>>

>>>> GCC ICEs under -mslow-flash-data and -mword-relocations because there

>>>> is no way to load an address, both literal pools and MOVW/MOVT being

>>>> forbidden. This patch gives an error message when both options are

>>>> specified by the user and adds the according dg-skip-if directives for

>>>> tests that use either of these options.

>>>>

>>>> ChangeLog entries are as follows:

>>>>

>>>> *** gcc/ChangeLog ***

>>>>

>>>> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

>>>>

>>>>        PR target/87374

>>>>        * config/arm/arm.c (arm_option_check_internal): Disable the combined

>>>>        use of -mslow-flash-data and -mword-relocations.

>>>>

>>>> *** gcc/testsuite/ChangeLog ***

>>>>

>>>> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

>>>>

>>>>        PR target/87374

>>>>        * gcc.target/arm/movdi_movt.c: Skip if both -mslow-flash-data and

>>>>        -mword-relocations would be passed when compiling the test.

>>>>        * gcc.target/arm/movsi_movt.c: Likewise.

>>>>        * gcc.target/arm/pr81863.c: Likewise.

>>>>        * gcc.target/arm/thumb2-slow-flash-data-1.c: Likewise.

>>>>        * gcc.target/arm/thumb2-slow-flash-data-2.c: Likewise.

>>>>        * gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.

>>>>        * gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.

>>>>        * gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.

>>>>        * gcc.target/arm/tls-disable-literal-pool.c: Likewise.

>>>>

>>>>

>>>> Testing: Bootstrapped in Thumb-2 mode. No testsuite regression when

>>>> targeting arm-none-eabi. Modified tests get skipped as expected when

>>>> running the testsuite with -mslow-flash-data (pr81863.c) or

>>>> -mword-relocations (all the others).

>>>>

>>>>

>>>> Is this ok for trunk? I'd also appreciate guidance on whether this is

>>>> worth a backport. It's a simple patch but on the other hand it only

>>>> prevents some option combination, it does not fix anything so I have

>>>> mixed feelings.

>>>

>>> In my opinion -mslow-flash-data is more of a tuning option rather than a security/ABI feature

>>> and therefore erroring out on its combination with -mword-relocations feels odd.

>>> I'm leaning more towards making -mword-relocations or any other option that really requires constant pools

>>> to bypass/disable the effects of -mslow-flash-data instead.

>>

>> -mslow-flash-data and -mword-relocations are contradictory in their

>> expectations. mslow-flash-data is for not putting anything in the

>> literal pool whereas mword-relocations is purely around the use of movw

>> / movt instructions for word sized values. I wish we had called

>> -mslow-flash-data something else (probably -mno-literal-pools).

>> -mslow-flash-data is used primarily by M-profile users and

>> -mword-relocations IIUC was a point fix for use in the Linux kernel for

>> module loads at a time when not all module loaders in the linux kernel

>> were fixed for the movw / movt relocations and armv7-a / thumb2 was in

>> it's infancy :). Thus they are used by different constituencies in

>> general and I wouldn't see them used together by actual users.

> 

> Technically, -mslow-flash-data does not forbid literal pool, it just

> discourages it because it's slower than many instructions. -mpure-code

> on the other hand reuse the same logic and does forbid literal pools.

> We could treat -mslow-flash-data differently but the question is

> whether it is worth the trouble.


Well, yeah I don't see the need for it. You could argue that 
-mslow-flash-data can be porous but realistically if you want this as an 
effective performance option , such porosity should be discouraged very 
strongly ;)

> 

> By the way, I've noticed that the documentation for -mword-relocations

> says it defaults to on for -fpic and -fPIC but when looking through

> the code I saw that target_word_relocation is not set in these case,

> rather the initial commit checks that introduced -mword-relocation

> also checks for flag_pic when checking target_word_relocation. However

> a later commit added one more check for target_word_relocations but

> nothing for flag_pic. I'm now consolidating this so that flag_pic sets

> target_word_relocations. I'll do a regression testing with -fPIC and

> then post an updated patch.


I'm reasonably sure that's *not* going to have *any* effect on code 
generation as in the -fpic / -fPIC case we always produce the funny GOT 
unspecs and have never used movw / movt instructions in those sequences 
for addressing. If that had happened most of the world's dynamic 
libraries would have faulted by now because I don't think they can 
process absolute movw / movt relocations.


It is automatically implied by the fact that we never produced PC 
relative versions of the immediates that get put into movw / movt 
instructions. I don't even remember us having effective relocations to 
implement this but this is going back a few years now.


Sure that probably needs a comment rather than being implicit from the 
source or from my own head :)

Ramana
Thomas Preudhomme Oct. 5, 2018, 4:50 p.m. | #6
Hi Ramana and Kyrill,

I've reworked the patch to add some documentation of the option
conflict and reworked the -mword-relocation logic slightly to set the
variable explicitely in PIC mode rather than test for PIC and word
relocation everywhere.

ChangeLog entries are now as follows:

*** gcc/ChangeLog ***

2018-10-02  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

    PR target/87374
    * config/arm/arm.c (arm_option_check_internal): Disable the combined
    use of -mslow-flash-data and -mword-relocations.
    (arm_option_override): Enable -mword-relocations if -fpic or -fPIC.
    * config/arm/arm.md (SYMBOL_REF MOVT splitter): Stop checking for
    flag_pic.
    * doc/invoke.texi (-mword-relocations): Mention conflict with
    -mslow-flash-data.
    (-mslow-flash-data): Reciprocally.

*** gcc/testsuite/ChangeLog ***

2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

    PR target/87374
    * gcc.target/arm/movdi_movt.c: Skip if both -mslow-flash-data and
    -mword-relocations would be passed when compiling the test.
    * gcc.target/arm/movsi_movt.c: Likewise.
    * gcc.target/arm/pr81863.c: Likewise.
    * gcc.target/arm/thumb2-slow-flash-data-1.c: Likewise.
    * gcc.target/arm/thumb2-slow-flash-data-2.c: Likewise.
    * gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.
    * gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.
    * gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.
    * gcc.target/arm/tls-disable-literal-pool.c: Likewise.

Is this ok for trunk?

Best regards,

Thomas

On Tue, 2 Oct 2018 at 13:39, Ramana Radhakrishnan
<ramana.radhakrishnan@foss.arm.com> wrote:
>

> On 02/10/2018 11:42, Thomas Preudhomme wrote:

> > Hi Ramana,

> >

> > On Thu, 27 Sep 2018 at 11:14, Ramana Radhakrishnan

> > <ramana.radhakrishnan@arm.com> wrote:

> >>

> >> On 27/09/2018 09:26, Kyrill Tkachov wrote:

> >>> Hi Thomas,

> >>>

> >>> On 26/09/18 18:39, Thomas Preudhomme wrote:

> >>>> Hi,

> >>>>

> >>>> GCC ICEs under -mslow-flash-data and -mword-relocations because there

> >>>> is no way to load an address, both literal pools and MOVW/MOVT being

> >>>> forbidden. This patch gives an error message when both options are

> >>>> specified by the user and adds the according dg-skip-if directives for

> >>>> tests that use either of these options.

> >>>>

> >>>> ChangeLog entries are as follows:

> >>>>

> >>>> *** gcc/ChangeLog ***

> >>>>

> >>>> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

> >>>>

> >>>>        PR target/87374

> >>>>        * config/arm/arm.c (arm_option_check_internal): Disable the combined

> >>>>        use of -mslow-flash-data and -mword-relocations.

> >>>>

> >>>> *** gcc/testsuite/ChangeLog ***

> >>>>

> >>>> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

> >>>>

> >>>>        PR target/87374

> >>>>        * gcc.target/arm/movdi_movt.c: Skip if both -mslow-flash-data and

> >>>>        -mword-relocations would be passed when compiling the test.

> >>>>        * gcc.target/arm/movsi_movt.c: Likewise.

> >>>>        * gcc.target/arm/pr81863.c: Likewise.

> >>>>        * gcc.target/arm/thumb2-slow-flash-data-1.c: Likewise.

> >>>>        * gcc.target/arm/thumb2-slow-flash-data-2.c: Likewise.

> >>>>        * gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.

> >>>>        * gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.

> >>>>        * gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.

> >>>>        * gcc.target/arm/tls-disable-literal-pool.c: Likewise.

> >>>>

> >>>>

> >>>> Testing: Bootstrapped in Thumb-2 mode. No testsuite regression when

> >>>> targeting arm-none-eabi. Modified tests get skipped as expected when

> >>>> running the testsuite with -mslow-flash-data (pr81863.c) or

> >>>> -mword-relocations (all the others).

> >>>>

> >>>>

> >>>> Is this ok for trunk? I'd also appreciate guidance on whether this is

> >>>> worth a backport. It's a simple patch but on the other hand it only

> >>>> prevents some option combination, it does not fix anything so I have

> >>>> mixed feelings.

> >>>

> >>> In my opinion -mslow-flash-data is more of a tuning option rather than a security/ABI feature

> >>> and therefore erroring out on its combination with -mword-relocations feels odd.

> >>> I'm leaning more towards making -mword-relocations or any other option that really requires constant pools

> >>> to bypass/disable the effects of -mslow-flash-data instead.

> >>

> >> -mslow-flash-data and -mword-relocations are contradictory in their

> >> expectations. mslow-flash-data is for not putting anything in the

> >> literal pool whereas mword-relocations is purely around the use of movw

> >> / movt instructions for word sized values. I wish we had called

> >> -mslow-flash-data something else (probably -mno-literal-pools).

> >> -mslow-flash-data is used primarily by M-profile users and

> >> -mword-relocations IIUC was a point fix for use in the Linux kernel for

> >> module loads at a time when not all module loaders in the linux kernel

> >> were fixed for the movw / movt relocations and armv7-a / thumb2 was in

> >> it's infancy :). Thus they are used by different constituencies in

> >> general and I wouldn't see them used together by actual users.

> >

> > Technically, -mslow-flash-data does not forbid literal pool, it just

> > discourages it because it's slower than many instructions. -mpure-code

> > on the other hand reuse the same logic and does forbid literal pools.

> > We could treat -mslow-flash-data differently but the question is

> > whether it is worth the trouble.

>

> Well, yeah I don't see the need for it. You could argue that

> -mslow-flash-data can be porous but realistically if you want this as an

> effective performance option , such porosity should be discouraged very

> strongly ;)

>

> >

> > By the way, I've noticed that the documentation for -mword-relocations

> > says it defaults to on for -fpic and -fPIC but when looking through

> > the code I saw that target_word_relocation is not set in these case,

> > rather the initial commit checks that introduced -mword-relocation

> > also checks for flag_pic when checking target_word_relocation. However

> > a later commit added one more check for target_word_relocations but

> > nothing for flag_pic. I'm now consolidating this so that flag_pic sets

> > target_word_relocations. I'll do a regression testing with -fPIC and

> > then post an updated patch.

>

> I'm reasonably sure that's *not* going to have *any* effect on code

> generation as in the -fpic / -fPIC case we always produce the funny GOT

> unspecs and have never used movw / movt instructions in those sequences

> for addressing. If that had happened most of the world's dynamic

> libraries would have faulted by now because I don't think they can

> process absolute movw / movt relocations.

>

>

> It is automatically implied by the fact that we never produced PC

> relative versions of the immediates that get put into movw / movt

> instructions. I don't even remember us having effective relocations to

> implement this but this is going back a few years now.

>

>

> Sure that probably needs a comment rather than being implicit from the

> source or from my own head :)

>

> Ramana
From d21e1e0c343f60e4e7de293b4c3cb0e87bf22f2f Mon Sep 17 00:00:00 2001
From: Thomas Preud'homme <thomas.preudhomme@linaro.org>
Date: Tue, 25 Sep 2018 15:55:10 +0100
Subject: [PATCH] [PATCH, GCC/ARM] Fix PR87374: ICE with -mslow-flash-data and
 -mword-relocations

Hi,

GCC ICEs under -mslow-flash-data and -mword-relocations because there
is no way to load an address, both literal pools and MOVW/MOVT being
forbidden. This patch gives an error message when both options are
specified by the user and adds the according dg-skip-if directives for
tests that use either of these options. It also explicitely set the
option when in PIC mode as per documentation rather than always check
for target_word_relocation together with flag_pic.

ChangeLog entries are as follows:

*** gcc/ChangeLog ***

2018-10-02  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

	PR target/87374
	* config/arm/arm.c (arm_option_check_internal): Disable the combined
	use of -mslow-flash-data and -mword-relocations.
	(arm_option_override): Enable -mword-relocations if -fpic or -fPIC.
	* config/arm/arm.md (SYMBOL_REF MOVT splitter): Stop checking for
	flag_pic.
	* doc/invoke.texi (-mword-relocations): Mention conflict with
	-mslow-flash-data.
	(-mslow-flash-data): Reciprocally.

*** gcc/testsuite/ChangeLog ***

2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

	PR target/87374
	* gcc.target/arm/movdi_movt.c: Skip if both -mslow-flash-data and
	-mword-relocations would be passed when compiling the test.
	* gcc.target/arm/movsi_movt.c: Likewise.
	* gcc.target/arm/pr81863.c: Likewise.
	* gcc.target/arm/thumb2-slow-flash-data-1.c: Likewise.
	* gcc.target/arm/thumb2-slow-flash-data-2.c: Likewise.
	* gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.
	* gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.
	* gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.
	* gcc.target/arm/tls-disable-literal-pool.c: Likewise.

Is this ok for trunk?

Best regards,

Thomas
---
 gcc/config/arm/arm.c                          | 22 +++++++++++++------
 gcc/config/arm/arm.md                         |  2 +-
 gcc/doc/invoke.texi                           |  4 ++--
 gcc/testsuite/gcc.target/arm/movdi_movt.c     |  1 +
 gcc/testsuite/gcc.target/arm/movsi_movt.c     |  1 +
 gcc/testsuite/gcc.target/arm/pr81863.c        |  1 +
 .../gcc.target/arm/thumb2-slow-flash-data-1.c |  1 +
 .../gcc.target/arm/thumb2-slow-flash-data-2.c |  1 +
 .../gcc.target/arm/thumb2-slow-flash-data-3.c |  1 +
 .../gcc.target/arm/thumb2-slow-flash-data-4.c |  1 +
 .../gcc.target/arm/thumb2-slow-flash-data-5.c |  1 +
 .../gcc.target/arm/tls-disable-literal-pool.c |  1 +
 12 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 6332e68df05..043bbe534a2 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -2893,17 +2893,22 @@ arm_option_check_internal (struct gcc_options *opts)
       flag_pic = 0;
     }
 
-  /* We only support -mpure-code and -mslow-flash-data on M-profile targets
-     with MOVT.  */
-  if ((target_pure_code || target_slow_flash_data)
-      && (!TARGET_HAVE_MOVT || arm_arch_notm || flag_pic || TARGET_NEON))
+  if (target_pure_code || target_slow_flash_data)
     {
       const char *flag = (target_pure_code ? "-mpure-code" :
 					     "-mslow-flash-data");
-      error ("%s only supports non-pic code on M-profile targets with the "
-	     "MOVT instruction", flag);
-    }
 
+      /* We only support -mpure-code and -mslow-flash-data on M-profile targets
+	 with MOVT.  */
+      if (!TARGET_HAVE_MOVT || arm_arch_notm || flag_pic || TARGET_NEON)
+	error ("%s only supports non-pic code on M-profile targets with the "
+	       "MOVT instruction", flag);
+
+      /* Cannot load addresses: -mslow-flash-data forbids literal pool and
+	 -mword-relocations forbids relocation of MOVT/MOVW.  */
+      if (target_word_relocations)
+	error ("%s incompatible with -mword-relocations", flag);
+    }
 }
 
 /* Recompute the global settings depending on target attribute options.  */
@@ -3489,6 +3494,9 @@ arm_option_override (void)
 	arm_pic_register = pic_register;
     }
 
+  if (flag_pic)
+    target_word_relocations = 1;
+
   /* Enable -mfix-cortex-m3-ldrd by default for Cortex-M3 cores.  */
   if (fix_cm3_ldrd == 2)
     {
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 270b8e454b3..a773518cefa 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -6128,7 +6128,7 @@
   [(set (match_operand:SI 0 "arm_general_register_operand" "")
        (match_operand:SI 1 "general_operand" ""))]
   "TARGET_USE_MOVT && GET_CODE (operands[1]) == SYMBOL_REF
-   && !flag_pic && !target_word_relocations
+   && !target_word_relocations
    && !arm_tls_referenced_p (operands[1])"
   [(clobber (const_int 0))]
 {
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 7ef4e7a449b..8030b911cc4 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -16964,7 +16964,7 @@ this option and always use the original scheme.
 Only generate absolute relocations on word-sized values (i.e. R_ARM_ABS32).
 This is enabled by default on targets (uClinux, SymbianOS) where the runtime
 loader imposes this restriction, and when @option{-fpic} or @option{-fPIC}
-is specified.
+is specified. This option conflicts with @option{-mslow-flash-data}.
 
 @item -mfix-cortex-m3-ldrd
 @opindex mfix-cortex-m3-ldrd
@@ -17001,7 +17001,7 @@ to Neon is high.
 Assume loading data from flash is slower than fetching instruction.
 Therefore literal load is minimized for better performance.
 This option is only supported when compiling for ARMv7 M-profile and
-off by default.
+off by default. It conflicts with @option{-mword-relocations}.
 
 @item -masm-syntax-unified
 @opindex masm-syntax-unified
diff --git a/gcc/testsuite/gcc.target/arm/movdi_movt.c b/gcc/testsuite/gcc.target/arm/movdi_movt.c
index e2a28ccbd99..a01ffa0dc93 100644
--- a/gcc/testsuite/gcc.target/arm/movdi_movt.c
+++ b/gcc/testsuite/gcc.target/arm/movdi_movt.c
@@ -1,4 +1,5 @@
 /* { dg-do compile { target { arm_cortex_m && { arm_thumb2_ok || arm_thumb1_movt_ok } } } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-O2 -mslow-flash-data" } */
 
 unsigned long long
diff --git a/gcc/testsuite/gcc.target/arm/movsi_movt.c b/gcc/testsuite/gcc.target/arm/movsi_movt.c
index 3cf46e2fd17..19d202ecd33 100644
--- a/gcc/testsuite/gcc.target/arm/movsi_movt.c
+++ b/gcc/testsuite/gcc.target/arm/movsi_movt.c
@@ -1,4 +1,5 @@
 /* { dg-do compile { target { arm_cortex_m && { arm_thumb2_ok || arm_thumb1_movt_ok } } } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-O2 -mslow-flash-data" } */
 
 unsigned
diff --git a/gcc/testsuite/gcc.target/arm/pr81863.c b/gcc/testsuite/gcc.target/arm/pr81863.c
index 63b1ed66b2c..225a0c5cc2b 100644
--- a/gcc/testsuite/gcc.target/arm/pr81863.c
+++ b/gcc/testsuite/gcc.target/arm/pr81863.c
@@ -1,5 +1,6 @@
 /* testsuite/gcc.target/arm/pr48183.c */
 /* { dg-do compile } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mslow-flash-data" } } */
 /* { dg-options "-O2 -mword-relocations -march=armv7-a -marm" } */
 /* { dg-final { scan-assembler-not "\[\\t \]+movw" } } */
 
diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c
index 089a72b67f3..d10391a69ac 100644
--- a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c
+++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c
@@ -6,6 +6,7 @@
 /* { dg-do compile } */
 /* { dg-require-effective-target arm_cortex_m } */
 /* { dg-require-effective-target arm_thumb2_ok } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-O2 -mthumb -mslow-flash-data" } */
 
 float sf;
diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c
index c87e050639d..90bd44e27e5 100644
--- a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c
+++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c
@@ -3,6 +3,7 @@
 /* { dg-require-effective-target arm_thumb2_ok } */
 /* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */
 /* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-march=armv7e-m+fp -mfloat-abi=hard -O2 -mthumb -mslow-flash-data" } */
 
 float f (float);
diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c
index 8c6210ee6c9..5d9cd9c4df2 100644
--- a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c
+++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c
@@ -3,6 +3,7 @@
 /* { dg-require-effective-target arm_thumb2_ok } */
 /* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */
 /* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-march=armv7e-m+fp -mfloat-abi=hard -mthumb -mslow-flash-data" } */
 
 /* From PR71607 */
diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c
index 1bcb6924ed2..0eeddd5e6ec 100644
--- a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c
+++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c
@@ -3,6 +3,7 @@
 /* { dg-require-effective-target arm_thumb2_ok } */
 /* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */
 /* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-march=armv7e-m+fp -mfloat-abi=hard -O2 -mthumb -mslow-flash-data" } */
 
 double __attribute__ ((target ("fpu=fpv5-d16")))
diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c
index 808fff05faa..7d52f3801b6 100644
--- a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c
+++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c
@@ -3,6 +3,7 @@
 /* { dg-require-effective-target arm_thumb2_ok } */
 /* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */
 /* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-march=armv7e-m+fp -mfloat-abi=hard -O2 -mthumb -mslow-flash-data" } */
 
 double __attribute__ ((target ("fpu=fpv5-sp-d16")))
diff --git a/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c b/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c
index 283201fdd97..834eaf6db92 100644
--- a/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c
+++ b/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c
@@ -2,6 +2,7 @@
 /* { dg-require-effective-target tls_native } */
 /* { dg-require-effective-target arm_cortex_m } */
 /* { dg-require-effective-target arm_thumb2_ok } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-mslow-flash-data" } */
 
 __thread int x = 0;
Thomas Preudhomme Oct. 15, 2018, 3:01 p.m. | #7
Ping?

Best regards,

Thomas
On Fri, 5 Oct 2018 at 17:50, Thomas Preudhomme
<thomas.preudhomme@linaro.org> wrote:
>

> Hi Ramana and Kyrill,

>

> I've reworked the patch to add some documentation of the option

> conflict and reworked the -mword-relocation logic slightly to set the

> variable explicitely in PIC mode rather than test for PIC and word

> relocation everywhere.

>

> ChangeLog entries are now as follows:

>

> *** gcc/ChangeLog ***

>

> 2018-10-02  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

>

>     PR target/87374

>     * config/arm/arm.c (arm_option_check_internal): Disable the combined

>     use of -mslow-flash-data and -mword-relocations.

>     (arm_option_override): Enable -mword-relocations if -fpic or -fPIC.

>     * config/arm/arm.md (SYMBOL_REF MOVT splitter): Stop checking for

>     flag_pic.

>     * doc/invoke.texi (-mword-relocations): Mention conflict with

>     -mslow-flash-data.

>     (-mslow-flash-data): Reciprocally.

>

> *** gcc/testsuite/ChangeLog ***

>

> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

>

>     PR target/87374

>     * gcc.target/arm/movdi_movt.c: Skip if both -mslow-flash-data and

>     -mword-relocations would be passed when compiling the test.

>     * gcc.target/arm/movsi_movt.c: Likewise.

>     * gcc.target/arm/pr81863.c: Likewise.

>     * gcc.target/arm/thumb2-slow-flash-data-1.c: Likewise.

>     * gcc.target/arm/thumb2-slow-flash-data-2.c: Likewise.

>     * gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.

>     * gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.

>     * gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.

>     * gcc.target/arm/tls-disable-literal-pool.c: Likewise.

>

> Is this ok for trunk?

>

> Best regards,

>

> Thomas

>

> On Tue, 2 Oct 2018 at 13:39, Ramana Radhakrishnan

> <ramana.radhakrishnan@foss.arm.com> wrote:

> >

> > On 02/10/2018 11:42, Thomas Preudhomme wrote:

> > > Hi Ramana,

> > >

> > > On Thu, 27 Sep 2018 at 11:14, Ramana Radhakrishnan

> > > <ramana.radhakrishnan@arm.com> wrote:

> > >>

> > >> On 27/09/2018 09:26, Kyrill Tkachov wrote:

> > >>> Hi Thomas,

> > >>>

> > >>> On 26/09/18 18:39, Thomas Preudhomme wrote:

> > >>>> Hi,

> > >>>>

> > >>>> GCC ICEs under -mslow-flash-data and -mword-relocations because there

> > >>>> is no way to load an address, both literal pools and MOVW/MOVT being

> > >>>> forbidden. This patch gives an error message when both options are

> > >>>> specified by the user and adds the according dg-skip-if directives for

> > >>>> tests that use either of these options.

> > >>>>

> > >>>> ChangeLog entries are as follows:

> > >>>>

> > >>>> *** gcc/ChangeLog ***

> > >>>>

> > >>>> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

> > >>>>

> > >>>>        PR target/87374

> > >>>>        * config/arm/arm.c (arm_option_check_internal): Disable the combined

> > >>>>        use of -mslow-flash-data and -mword-relocations.

> > >>>>

> > >>>> *** gcc/testsuite/ChangeLog ***

> > >>>>

> > >>>> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

> > >>>>

> > >>>>        PR target/87374

> > >>>>        * gcc.target/arm/movdi_movt.c: Skip if both -mslow-flash-data and

> > >>>>        -mword-relocations would be passed when compiling the test.

> > >>>>        * gcc.target/arm/movsi_movt.c: Likewise.

> > >>>>        * gcc.target/arm/pr81863.c: Likewise.

> > >>>>        * gcc.target/arm/thumb2-slow-flash-data-1.c: Likewise.

> > >>>>        * gcc.target/arm/thumb2-slow-flash-data-2.c: Likewise.

> > >>>>        * gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.

> > >>>>        * gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.

> > >>>>        * gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.

> > >>>>        * gcc.target/arm/tls-disable-literal-pool.c: Likewise.

> > >>>>

> > >>>>

> > >>>> Testing: Bootstrapped in Thumb-2 mode. No testsuite regression when

> > >>>> targeting arm-none-eabi. Modified tests get skipped as expected when

> > >>>> running the testsuite with -mslow-flash-data (pr81863.c) or

> > >>>> -mword-relocations (all the others).

> > >>>>

> > >>>>

> > >>>> Is this ok for trunk? I'd also appreciate guidance on whether this is

> > >>>> worth a backport. It's a simple patch but on the other hand it only

> > >>>> prevents some option combination, it does not fix anything so I have

> > >>>> mixed feelings.

> > >>>

> > >>> In my opinion -mslow-flash-data is more of a tuning option rather than a security/ABI feature

> > >>> and therefore erroring out on its combination with -mword-relocations feels odd.

> > >>> I'm leaning more towards making -mword-relocations or any other option that really requires constant pools

> > >>> to bypass/disable the effects of -mslow-flash-data instead.

> > >>

> > >> -mslow-flash-data and -mword-relocations are contradictory in their

> > >> expectations. mslow-flash-data is for not putting anything in the

> > >> literal pool whereas mword-relocations is purely around the use of movw

> > >> / movt instructions for word sized values. I wish we had called

> > >> -mslow-flash-data something else (probably -mno-literal-pools).

> > >> -mslow-flash-data is used primarily by M-profile users and

> > >> -mword-relocations IIUC was a point fix for use in the Linux kernel for

> > >> module loads at a time when not all module loaders in the linux kernel

> > >> were fixed for the movw / movt relocations and armv7-a / thumb2 was in

> > >> it's infancy :). Thus they are used by different constituencies in

> > >> general and I wouldn't see them used together by actual users.

> > >

> > > Technically, -mslow-flash-data does not forbid literal pool, it just

> > > discourages it because it's slower than many instructions. -mpure-code

> > > on the other hand reuse the same logic and does forbid literal pools.

> > > We could treat -mslow-flash-data differently but the question is

> > > whether it is worth the trouble.

> >

> > Well, yeah I don't see the need for it. You could argue that

> > -mslow-flash-data can be porous but realistically if you want this as an

> > effective performance option , such porosity should be discouraged very

> > strongly ;)

> >

> > >

> > > By the way, I've noticed that the documentation for -mword-relocations

> > > says it defaults to on for -fpic and -fPIC but when looking through

> > > the code I saw that target_word_relocation is not set in these case,

> > > rather the initial commit checks that introduced -mword-relocation

> > > also checks for flag_pic when checking target_word_relocation. However

> > > a later commit added one more check for target_word_relocations but

> > > nothing for flag_pic. I'm now consolidating this so that flag_pic sets

> > > target_word_relocations. I'll do a regression testing with -fPIC and

> > > then post an updated patch.

> >

> > I'm reasonably sure that's *not* going to have *any* effect on code

> > generation as in the -fpic / -fPIC case we always produce the funny GOT

> > unspecs and have never used movw / movt instructions in those sequences

> > for addressing. If that had happened most of the world's dynamic

> > libraries would have faulted by now because I don't think they can

> > process absolute movw / movt relocations.

> >

> >

> > It is automatically implied by the fact that we never produced PC

> > relative versions of the immediates that get put into movw / movt

> > instructions. I don't even remember us having effective relocations to

> > implement this but this is going back a few years now.

> >

> >

> > Sure that probably needs a comment rather than being implicit from the

> > source or from my own head :)

> >

> > Ramana
Thomas Preudhomme Oct. 23, 2018, 9:10 a.m. | #8
Ping?

Best regards,

Thomas

On Mon, 15 Oct 2018 at 16:01, Thomas Preudhomme
<thomas.preudhomme@linaro.org> wrote:
>

> Ping?

>

> Best regards,

>

> Thomas

> On Fri, 5 Oct 2018 at 17:50, Thomas Preudhomme

> <thomas.preudhomme@linaro.org> wrote:

> >

> > Hi Ramana and Kyrill,

> >

> > I've reworked the patch to add some documentation of the option

> > conflict and reworked the -mword-relocation logic slightly to set the

> > variable explicitely in PIC mode rather than test for PIC and word

> > relocation everywhere.

> >

> > ChangeLog entries are now as follows:

> >

> > *** gcc/ChangeLog ***

> >

> > 2018-10-02  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

> >

> >     PR target/87374

> >     * config/arm/arm.c (arm_option_check_internal): Disable the combined

> >     use of -mslow-flash-data and -mword-relocations.

> >     (arm_option_override): Enable -mword-relocations if -fpic or -fPIC.

> >     * config/arm/arm.md (SYMBOL_REF MOVT splitter): Stop checking for

> >     flag_pic.

> >     * doc/invoke.texi (-mword-relocations): Mention conflict with

> >     -mslow-flash-data.

> >     (-mslow-flash-data): Reciprocally.

> >

> > *** gcc/testsuite/ChangeLog ***

> >

> > 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

> >

> >     PR target/87374

> >     * gcc.target/arm/movdi_movt.c: Skip if both -mslow-flash-data and

> >     -mword-relocations would be passed when compiling the test.

> >     * gcc.target/arm/movsi_movt.c: Likewise.

> >     * gcc.target/arm/pr81863.c: Likewise.

> >     * gcc.target/arm/thumb2-slow-flash-data-1.c: Likewise.

> >     * gcc.target/arm/thumb2-slow-flash-data-2.c: Likewise.

> >     * gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.

> >     * gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.

> >     * gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.

> >     * gcc.target/arm/tls-disable-literal-pool.c: Likewise.

> >

> > Is this ok for trunk?

> >

> > Best regards,

> >

> > Thomas

> >

> > On Tue, 2 Oct 2018 at 13:39, Ramana Radhakrishnan

> > <ramana.radhakrishnan@foss.arm.com> wrote:

> > >

> > > On 02/10/2018 11:42, Thomas Preudhomme wrote:

> > > > Hi Ramana,

> > > >

> > > > On Thu, 27 Sep 2018 at 11:14, Ramana Radhakrishnan

> > > > <ramana.radhakrishnan@arm.com> wrote:

> > > >>

> > > >> On 27/09/2018 09:26, Kyrill Tkachov wrote:

> > > >>> Hi Thomas,

> > > >>>

> > > >>> On 26/09/18 18:39, Thomas Preudhomme wrote:

> > > >>>> Hi,

> > > >>>>

> > > >>>> GCC ICEs under -mslow-flash-data and -mword-relocations because there

> > > >>>> is no way to load an address, both literal pools and MOVW/MOVT being

> > > >>>> forbidden. This patch gives an error message when both options are

> > > >>>> specified by the user and adds the according dg-skip-if directives for

> > > >>>> tests that use either of these options.

> > > >>>>

> > > >>>> ChangeLog entries are as follows:

> > > >>>>

> > > >>>> *** gcc/ChangeLog ***

> > > >>>>

> > > >>>> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

> > > >>>>

> > > >>>>        PR target/87374

> > > >>>>        * config/arm/arm.c (arm_option_check_internal): Disable the combined

> > > >>>>        use of -mslow-flash-data and -mword-relocations.

> > > >>>>

> > > >>>> *** gcc/testsuite/ChangeLog ***

> > > >>>>

> > > >>>> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

> > > >>>>

> > > >>>>        PR target/87374

> > > >>>>        * gcc.target/arm/movdi_movt.c: Skip if both -mslow-flash-data and

> > > >>>>        -mword-relocations would be passed when compiling the test.

> > > >>>>        * gcc.target/arm/movsi_movt.c: Likewise.

> > > >>>>        * gcc.target/arm/pr81863.c: Likewise.

> > > >>>>        * gcc.target/arm/thumb2-slow-flash-data-1.c: Likewise.

> > > >>>>        * gcc.target/arm/thumb2-slow-flash-data-2.c: Likewise.

> > > >>>>        * gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.

> > > >>>>        * gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.

> > > >>>>        * gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.

> > > >>>>        * gcc.target/arm/tls-disable-literal-pool.c: Likewise.

> > > >>>>

> > > >>>>

> > > >>>> Testing: Bootstrapped in Thumb-2 mode. No testsuite regression when

> > > >>>> targeting arm-none-eabi. Modified tests get skipped as expected when

> > > >>>> running the testsuite with -mslow-flash-data (pr81863.c) or

> > > >>>> -mword-relocations (all the others).

> > > >>>>

> > > >>>>

> > > >>>> Is this ok for trunk? I'd also appreciate guidance on whether this is

> > > >>>> worth a backport. It's a simple patch but on the other hand it only

> > > >>>> prevents some option combination, it does not fix anything so I have

> > > >>>> mixed feelings.

> > > >>>

> > > >>> In my opinion -mslow-flash-data is more of a tuning option rather than a security/ABI feature

> > > >>> and therefore erroring out on its combination with -mword-relocations feels odd.

> > > >>> I'm leaning more towards making -mword-relocations or any other option that really requires constant pools

> > > >>> to bypass/disable the effects of -mslow-flash-data instead.

> > > >>

> > > >> -mslow-flash-data and -mword-relocations are contradictory in their

> > > >> expectations. mslow-flash-data is for not putting anything in the

> > > >> literal pool whereas mword-relocations is purely around the use of movw

> > > >> / movt instructions for word sized values. I wish we had called

> > > >> -mslow-flash-data something else (probably -mno-literal-pools).

> > > >> -mslow-flash-data is used primarily by M-profile users and

> > > >> -mword-relocations IIUC was a point fix for use in the Linux kernel for

> > > >> module loads at a time when not all module loaders in the linux kernel

> > > >> were fixed for the movw / movt relocations and armv7-a / thumb2 was in

> > > >> it's infancy :). Thus they are used by different constituencies in

> > > >> general and I wouldn't see them used together by actual users.

> > > >

> > > > Technically, -mslow-flash-data does not forbid literal pool, it just

> > > > discourages it because it's slower than many instructions. -mpure-code

> > > > on the other hand reuse the same logic and does forbid literal pools.

> > > > We could treat -mslow-flash-data differently but the question is

> > > > whether it is worth the trouble.

> > >

> > > Well, yeah I don't see the need for it. You could argue that

> > > -mslow-flash-data can be porous but realistically if you want this as an

> > > effective performance option , such porosity should be discouraged very

> > > strongly ;)

> > >

> > > >

> > > > By the way, I've noticed that the documentation for -mword-relocations

> > > > says it defaults to on for -fpic and -fPIC but when looking through

> > > > the code I saw that target_word_relocation is not set in these case,

> > > > rather the initial commit checks that introduced -mword-relocation

> > > > also checks for flag_pic when checking target_word_relocation. However

> > > > a later commit added one more check for target_word_relocations but

> > > > nothing for flag_pic. I'm now consolidating this so that flag_pic sets

> > > > target_word_relocations. I'll do a regression testing with -fPIC and

> > > > then post an updated patch.

> > >

> > > I'm reasonably sure that's *not* going to have *any* effect on code

> > > generation as in the -fpic / -fPIC case we always produce the funny GOT

> > > unspecs and have never used movw / movt instructions in those sequences

> > > for addressing. If that had happened most of the world's dynamic

> > > libraries would have faulted by now because I don't think they can

> > > process absolute movw / movt relocations.

> > >

> > >

> > > It is automatically implied by the fact that we never produced PC

> > > relative versions of the immediates that get put into movw / movt

> > > instructions. I don't even remember us having effective relocations to

> > > implement this but this is going back a few years now.

> > >

> > >

> > > Sure that probably needs a comment rather than being implicit from the

> > > source or from my own head :)

> > >

> > > Ramana
From d21e1e0c343f60e4e7de293b4c3cb0e87bf22f2f Mon Sep 17 00:00:00 2001
From: Thomas Preud'homme <thomas.preudhomme@linaro.org>
Date: Tue, 25 Sep 2018 15:55:10 +0100
Subject: [PATCH] [PATCH, GCC/ARM] Fix PR87374: ICE with -mslow-flash-data and
 -mword-relocations

Hi,

GCC ICEs under -mslow-flash-data and -mword-relocations because there
is no way to load an address, both literal pools and MOVW/MOVT being
forbidden. This patch gives an error message when both options are
specified by the user and adds the according dg-skip-if directives for
tests that use either of these options. It also explicitely set the
option when in PIC mode as per documentation rather than always check
for target_word_relocation together with flag_pic.

ChangeLog entries are as follows:

*** gcc/ChangeLog ***

2018-10-02  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

	PR target/87374
	* config/arm/arm.c (arm_option_check_internal): Disable the combined
	use of -mslow-flash-data and -mword-relocations.
	(arm_option_override): Enable -mword-relocations if -fpic or -fPIC.
	* config/arm/arm.md (SYMBOL_REF MOVT splitter): Stop checking for
	flag_pic.
	* doc/invoke.texi (-mword-relocations): Mention conflict with
	-mslow-flash-data.
	(-mslow-flash-data): Reciprocally.

*** gcc/testsuite/ChangeLog ***

2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

	PR target/87374
	* gcc.target/arm/movdi_movt.c: Skip if both -mslow-flash-data and
	-mword-relocations would be passed when compiling the test.
	* gcc.target/arm/movsi_movt.c: Likewise.
	* gcc.target/arm/pr81863.c: Likewise.
	* gcc.target/arm/thumb2-slow-flash-data-1.c: Likewise.
	* gcc.target/arm/thumb2-slow-flash-data-2.c: Likewise.
	* gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.
	* gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.
	* gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.
	* gcc.target/arm/tls-disable-literal-pool.c: Likewise.

Is this ok for trunk?

Best regards,

Thomas
---
 gcc/config/arm/arm.c                          | 22 +++++++++++++------
 gcc/config/arm/arm.md                         |  2 +-
 gcc/doc/invoke.texi                           |  4 ++--
 gcc/testsuite/gcc.target/arm/movdi_movt.c     |  1 +
 gcc/testsuite/gcc.target/arm/movsi_movt.c     |  1 +
 gcc/testsuite/gcc.target/arm/pr81863.c        |  1 +
 .../gcc.target/arm/thumb2-slow-flash-data-1.c |  1 +
 .../gcc.target/arm/thumb2-slow-flash-data-2.c |  1 +
 .../gcc.target/arm/thumb2-slow-flash-data-3.c |  1 +
 .../gcc.target/arm/thumb2-slow-flash-data-4.c |  1 +
 .../gcc.target/arm/thumb2-slow-flash-data-5.c |  1 +
 .../gcc.target/arm/tls-disable-literal-pool.c |  1 +
 12 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 6332e68df05..043bbe534a2 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -2893,17 +2893,22 @@ arm_option_check_internal (struct gcc_options *opts)
       flag_pic = 0;
     }
 
-  /* We only support -mpure-code and -mslow-flash-data on M-profile targets
-     with MOVT.  */
-  if ((target_pure_code || target_slow_flash_data)
-      && (!TARGET_HAVE_MOVT || arm_arch_notm || flag_pic || TARGET_NEON))
+  if (target_pure_code || target_slow_flash_data)
     {
       const char *flag = (target_pure_code ? "-mpure-code" :
 					     "-mslow-flash-data");
-      error ("%s only supports non-pic code on M-profile targets with the "
-	     "MOVT instruction", flag);
-    }
 
+      /* We only support -mpure-code and -mslow-flash-data on M-profile targets
+	 with MOVT.  */
+      if (!TARGET_HAVE_MOVT || arm_arch_notm || flag_pic || TARGET_NEON)
+	error ("%s only supports non-pic code on M-profile targets with the "
+	       "MOVT instruction", flag);
+
+      /* Cannot load addresses: -mslow-flash-data forbids literal pool and
+	 -mword-relocations forbids relocation of MOVT/MOVW.  */
+      if (target_word_relocations)
+	error ("%s incompatible with -mword-relocations", flag);
+    }
 }
 
 /* Recompute the global settings depending on target attribute options.  */
@@ -3489,6 +3494,9 @@ arm_option_override (void)
 	arm_pic_register = pic_register;
     }
 
+  if (flag_pic)
+    target_word_relocations = 1;
+
   /* Enable -mfix-cortex-m3-ldrd by default for Cortex-M3 cores.  */
   if (fix_cm3_ldrd == 2)
     {
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 270b8e454b3..a773518cefa 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -6128,7 +6128,7 @@
   [(set (match_operand:SI 0 "arm_general_register_operand" "")
        (match_operand:SI 1 "general_operand" ""))]
   "TARGET_USE_MOVT && GET_CODE (operands[1]) == SYMBOL_REF
-   && !flag_pic && !target_word_relocations
+   && !target_word_relocations
    && !arm_tls_referenced_p (operands[1])"
   [(clobber (const_int 0))]
 {
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 7ef4e7a449b..8030b911cc4 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -16964,7 +16964,7 @@ this option and always use the original scheme.
 Only generate absolute relocations on word-sized values (i.e. R_ARM_ABS32).
 This is enabled by default on targets (uClinux, SymbianOS) where the runtime
 loader imposes this restriction, and when @option{-fpic} or @option{-fPIC}
-is specified.
+is specified. This option conflicts with @option{-mslow-flash-data}.
 
 @item -mfix-cortex-m3-ldrd
 @opindex mfix-cortex-m3-ldrd
@@ -17001,7 +17001,7 @@ to Neon is high.
 Assume loading data from flash is slower than fetching instruction.
 Therefore literal load is minimized for better performance.
 This option is only supported when compiling for ARMv7 M-profile and
-off by default.
+off by default. It conflicts with @option{-mword-relocations}.
 
 @item -masm-syntax-unified
 @opindex masm-syntax-unified
diff --git a/gcc/testsuite/gcc.target/arm/movdi_movt.c b/gcc/testsuite/gcc.target/arm/movdi_movt.c
index e2a28ccbd99..a01ffa0dc93 100644
--- a/gcc/testsuite/gcc.target/arm/movdi_movt.c
+++ b/gcc/testsuite/gcc.target/arm/movdi_movt.c
@@ -1,4 +1,5 @@
 /* { dg-do compile { target { arm_cortex_m && { arm_thumb2_ok || arm_thumb1_movt_ok } } } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-O2 -mslow-flash-data" } */
 
 unsigned long long
diff --git a/gcc/testsuite/gcc.target/arm/movsi_movt.c b/gcc/testsuite/gcc.target/arm/movsi_movt.c
index 3cf46e2fd17..19d202ecd33 100644
--- a/gcc/testsuite/gcc.target/arm/movsi_movt.c
+++ b/gcc/testsuite/gcc.target/arm/movsi_movt.c
@@ -1,4 +1,5 @@
 /* { dg-do compile { target { arm_cortex_m && { arm_thumb2_ok || arm_thumb1_movt_ok } } } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-O2 -mslow-flash-data" } */
 
 unsigned
diff --git a/gcc/testsuite/gcc.target/arm/pr81863.c b/gcc/testsuite/gcc.target/arm/pr81863.c
index 63b1ed66b2c..225a0c5cc2b 100644
--- a/gcc/testsuite/gcc.target/arm/pr81863.c
+++ b/gcc/testsuite/gcc.target/arm/pr81863.c
@@ -1,5 +1,6 @@
 /* testsuite/gcc.target/arm/pr48183.c */
 /* { dg-do compile } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mslow-flash-data" } } */
 /* { dg-options "-O2 -mword-relocations -march=armv7-a -marm" } */
 /* { dg-final { scan-assembler-not "\[\\t \]+movw" } } */
 
diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c
index 089a72b67f3..d10391a69ac 100644
--- a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c
+++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c
@@ -6,6 +6,7 @@
 /* { dg-do compile } */
 /* { dg-require-effective-target arm_cortex_m } */
 /* { dg-require-effective-target arm_thumb2_ok } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-O2 -mthumb -mslow-flash-data" } */
 
 float sf;
diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c
index c87e050639d..90bd44e27e5 100644
--- a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c
+++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c
@@ -3,6 +3,7 @@
 /* { dg-require-effective-target arm_thumb2_ok } */
 /* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */
 /* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-march=armv7e-m+fp -mfloat-abi=hard -O2 -mthumb -mslow-flash-data" } */
 
 float f (float);
diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c
index 8c6210ee6c9..5d9cd9c4df2 100644
--- a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c
+++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c
@@ -3,6 +3,7 @@
 /* { dg-require-effective-target arm_thumb2_ok } */
 /* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */
 /* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-march=armv7e-m+fp -mfloat-abi=hard -mthumb -mslow-flash-data" } */
 
 /* From PR71607 */
diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c
index 1bcb6924ed2..0eeddd5e6ec 100644
--- a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c
+++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c
@@ -3,6 +3,7 @@
 /* { dg-require-effective-target arm_thumb2_ok } */
 /* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */
 /* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-march=armv7e-m+fp -mfloat-abi=hard -O2 -mthumb -mslow-flash-data" } */
 
 double __attribute__ ((target ("fpu=fpv5-d16")))
diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c
index 808fff05faa..7d52f3801b6 100644
--- a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c
+++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c
@@ -3,6 +3,7 @@
 /* { dg-require-effective-target arm_thumb2_ok } */
 /* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */
 /* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-march=armv7e-m+fp -mfloat-abi=hard -O2 -mthumb -mslow-flash-data" } */
 
 double __attribute__ ((target ("fpu=fpv5-sp-d16")))
diff --git a/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c b/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c
index 283201fdd97..834eaf6db92 100644
--- a/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c
+++ b/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c
@@ -2,6 +2,7 @@
 /* { dg-require-effective-target tls_native } */
 /* { dg-require-effective-target arm_cortex_m } */
 /* { dg-require-effective-target arm_thumb2_ok } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-mslow-flash-data" } */
 
 __thread int x = 0;
Thomas Preudhomme Oct. 30, 2018, 10:54 a.m. | #9
Ping?

Best regards,

Thomas

On Tue, 23 Oct 2018 at 10:10, Thomas Preudhomme
<thomas.preudhomme@linaro.org> wrote:
>

> Ping?

>

> Best regards,

>

> Thomas

>

> On Mon, 15 Oct 2018 at 16:01, Thomas Preudhomme

> <thomas.preudhomme@linaro.org> wrote:

> >

> > Ping?

> >

> > Best regards,

> >

> > Thomas

> > On Fri, 5 Oct 2018 at 17:50, Thomas Preudhomme

> > <thomas.preudhomme@linaro.org> wrote:

> > >

> > > Hi Ramana and Kyrill,

> > >

> > > I've reworked the patch to add some documentation of the option

> > > conflict and reworked the -mword-relocation logic slightly to set the

> > > variable explicitely in PIC mode rather than test for PIC and word

> > > relocation everywhere.

> > >

> > > ChangeLog entries are now as follows:

> > >

> > > *** gcc/ChangeLog ***

> > >

> > > 2018-10-02  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

> > >

> > >     PR target/87374

> > >     * config/arm/arm.c (arm_option_check_internal): Disable the combined

> > >     use of -mslow-flash-data and -mword-relocations.

> > >     (arm_option_override): Enable -mword-relocations if -fpic or -fPIC.

> > >     * config/arm/arm.md (SYMBOL_REF MOVT splitter): Stop checking for

> > >     flag_pic.

> > >     * doc/invoke.texi (-mword-relocations): Mention conflict with

> > >     -mslow-flash-data.

> > >     (-mslow-flash-data): Reciprocally.

> > >

> > > *** gcc/testsuite/ChangeLog ***

> > >

> > > 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

> > >

> > >     PR target/87374

> > >     * gcc.target/arm/movdi_movt.c: Skip if both -mslow-flash-data and

> > >     -mword-relocations would be passed when compiling the test.

> > >     * gcc.target/arm/movsi_movt.c: Likewise.

> > >     * gcc.target/arm/pr81863.c: Likewise.

> > >     * gcc.target/arm/thumb2-slow-flash-data-1.c: Likewise.

> > >     * gcc.target/arm/thumb2-slow-flash-data-2.c: Likewise.

> > >     * gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.

> > >     * gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.

> > >     * gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.

> > >     * gcc.target/arm/tls-disable-literal-pool.c: Likewise.

> > >

> > > Is this ok for trunk?

> > >

> > > Best regards,

> > >

> > > Thomas

> > >

> > > On Tue, 2 Oct 2018 at 13:39, Ramana Radhakrishnan

> > > <ramana.radhakrishnan@foss.arm.com> wrote:

> > > >

> > > > On 02/10/2018 11:42, Thomas Preudhomme wrote:

> > > > > Hi Ramana,

> > > > >

> > > > > On Thu, 27 Sep 2018 at 11:14, Ramana Radhakrishnan

> > > > > <ramana.radhakrishnan@arm.com> wrote:

> > > > >>

> > > > >> On 27/09/2018 09:26, Kyrill Tkachov wrote:

> > > > >>> Hi Thomas,

> > > > >>>

> > > > >>> On 26/09/18 18:39, Thomas Preudhomme wrote:

> > > > >>>> Hi,

> > > > >>>>

> > > > >>>> GCC ICEs under -mslow-flash-data and -mword-relocations because there

> > > > >>>> is no way to load an address, both literal pools and MOVW/MOVT being

> > > > >>>> forbidden. This patch gives an error message when both options are

> > > > >>>> specified by the user and adds the according dg-skip-if directives for

> > > > >>>> tests that use either of these options.

> > > > >>>>

> > > > >>>> ChangeLog entries are as follows:

> > > > >>>>

> > > > >>>> *** gcc/ChangeLog ***

> > > > >>>>

> > > > >>>> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

> > > > >>>>

> > > > >>>>        PR target/87374

> > > > >>>>        * config/arm/arm.c (arm_option_check_internal): Disable the combined

> > > > >>>>        use of -mslow-flash-data and -mword-relocations.

> > > > >>>>

> > > > >>>> *** gcc/testsuite/ChangeLog ***

> > > > >>>>

> > > > >>>> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

> > > > >>>>

> > > > >>>>        PR target/87374

> > > > >>>>        * gcc.target/arm/movdi_movt.c: Skip if both -mslow-flash-data and

> > > > >>>>        -mword-relocations would be passed when compiling the test.

> > > > >>>>        * gcc.target/arm/movsi_movt.c: Likewise.

> > > > >>>>        * gcc.target/arm/pr81863.c: Likewise.

> > > > >>>>        * gcc.target/arm/thumb2-slow-flash-data-1.c: Likewise.

> > > > >>>>        * gcc.target/arm/thumb2-slow-flash-data-2.c: Likewise.

> > > > >>>>        * gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.

> > > > >>>>        * gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.

> > > > >>>>        * gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.

> > > > >>>>        * gcc.target/arm/tls-disable-literal-pool.c: Likewise.

> > > > >>>>

> > > > >>>>

> > > > >>>> Testing: Bootstrapped in Thumb-2 mode. No testsuite regression when

> > > > >>>> targeting arm-none-eabi. Modified tests get skipped as expected when

> > > > >>>> running the testsuite with -mslow-flash-data (pr81863.c) or

> > > > >>>> -mword-relocations (all the others).

> > > > >>>>

> > > > >>>>

> > > > >>>> Is this ok for trunk? I'd also appreciate guidance on whether this is

> > > > >>>> worth a backport. It's a simple patch but on the other hand it only

> > > > >>>> prevents some option combination, it does not fix anything so I have

> > > > >>>> mixed feelings.

> > > > >>>

> > > > >>> In my opinion -mslow-flash-data is more of a tuning option rather than a security/ABI feature

> > > > >>> and therefore erroring out on its combination with -mword-relocations feels odd.

> > > > >>> I'm leaning more towards making -mword-relocations or any other option that really requires constant pools

> > > > >>> to bypass/disable the effects of -mslow-flash-data instead.

> > > > >>

> > > > >> -mslow-flash-data and -mword-relocations are contradictory in their

> > > > >> expectations. mslow-flash-data is for not putting anything in the

> > > > >> literal pool whereas mword-relocations is purely around the use of movw

> > > > >> / movt instructions for word sized values. I wish we had called

> > > > >> -mslow-flash-data something else (probably -mno-literal-pools).

> > > > >> -mslow-flash-data is used primarily by M-profile users and

> > > > >> -mword-relocations IIUC was a point fix for use in the Linux kernel for

> > > > >> module loads at a time when not all module loaders in the linux kernel

> > > > >> were fixed for the movw / movt relocations and armv7-a / thumb2 was in

> > > > >> it's infancy :). Thus they are used by different constituencies in

> > > > >> general and I wouldn't see them used together by actual users.

> > > > >

> > > > > Technically, -mslow-flash-data does not forbid literal pool, it just

> > > > > discourages it because it's slower than many instructions. -mpure-code

> > > > > on the other hand reuse the same logic and does forbid literal pools.

> > > > > We could treat -mslow-flash-data differently but the question is

> > > > > whether it is worth the trouble.

> > > >

> > > > Well, yeah I don't see the need for it. You could argue that

> > > > -mslow-flash-data can be porous but realistically if you want this as an

> > > > effective performance option , such porosity should be discouraged very

> > > > strongly ;)

> > > >

> > > > >

> > > > > By the way, I've noticed that the documentation for -mword-relocations

> > > > > says it defaults to on for -fpic and -fPIC but when looking through

> > > > > the code I saw that target_word_relocation is not set in these case,

> > > > > rather the initial commit checks that introduced -mword-relocation

> > > > > also checks for flag_pic when checking target_word_relocation. However

> > > > > a later commit added one more check for target_word_relocations but

> > > > > nothing for flag_pic. I'm now consolidating this so that flag_pic sets

> > > > > target_word_relocations. I'll do a regression testing with -fPIC and

> > > > > then post an updated patch.

> > > >

> > > > I'm reasonably sure that's *not* going to have *any* effect on code

> > > > generation as in the -fpic / -fPIC case we always produce the funny GOT

> > > > unspecs and have never used movw / movt instructions in those sequences

> > > > for addressing. If that had happened most of the world's dynamic

> > > > libraries would have faulted by now because I don't think they can

> > > > process absolute movw / movt relocations.

> > > >

> > > >

> > > > It is automatically implied by the fact that we never produced PC

> > > > relative versions of the immediates that get put into movw / movt

> > > > instructions. I don't even remember us having effective relocations to

> > > > implement this but this is going back a few years now.

> > > >

> > > >

> > > > Sure that probably needs a comment rather than being implicit from the

> > > > source or from my own head :)

> > > >

> > > > Ramana
Ramana Radhakrishnan Oct. 30, 2018, 3:34 p.m. | #10
On Fri, Oct 5, 2018 at 5:50 PM Thomas Preudhomme
<thomas.preudhomme@linaro.org> wrote:
>

> Hi Ramana and Kyrill,

>

> I've reworked the patch to add some documentation of the option

> conflict and reworked the -mword-relocation logic slightly to set the

> variable explicitely in PIC mode rather than test for PIC and word

> relocation everywhere.


Ok.

Thanks,
Ramana

>

> ChangeLog entries are now as follows:

>

> *** gcc/ChangeLog ***

>

> 2018-10-02  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

>

>     PR target/87374

>     * config/arm/arm.c (arm_option_check_internal): Disable the combined

>     use of -mslow-flash-data and -mword-relocations.

>     (arm_option_override): Enable -mword-relocations if -fpic or -fPIC.

>     * config/arm/arm.md (SYMBOL_REF MOVT splitter): Stop checking for

>     flag_pic.

>     * doc/invoke.texi (-mword-relocations): Mention conflict with

>     -mslow-flash-data.

>     (-mslow-flash-data): Reciprocally.

>

> *** gcc/testsuite/ChangeLog ***

>

> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

>

>     PR target/87374

>     * gcc.target/arm/movdi_movt.c: Skip if both -mslow-flash-data and

>     -mword-relocations would be passed when compiling the test.

>     * gcc.target/arm/movsi_movt.c: Likewise.

>     * gcc.target/arm/pr81863.c: Likewise.

>     * gcc.target/arm/thumb2-slow-flash-data-1.c: Likewise.

>     * gcc.target/arm/thumb2-slow-flash-data-2.c: Likewise.

>     * gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.

>     * gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.

>     * gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.

>     * gcc.target/arm/tls-disable-literal-pool.c: Likewise.

>

> Is this ok for trunk?

>

> Best regards,

>

> Thomas

>

> On Tue, 2 Oct 2018 at 13:39, Ramana Radhakrishnan

> <ramana.radhakrishnan@foss.arm.com> wrote:

> >

> > On 02/10/2018 11:42, Thomas Preudhomme wrote:

> > > Hi Ramana,

> > >

> > > On Thu, 27 Sep 2018 at 11:14, Ramana Radhakrishnan

> > > <ramana.radhakrishnan@arm.com> wrote:

> > >>

> > >> On 27/09/2018 09:26, Kyrill Tkachov wrote:

> > >>> Hi Thomas,

> > >>>

> > >>> On 26/09/18 18:39, Thomas Preudhomme wrote:

> > >>>> Hi,

> > >>>>

> > >>>> GCC ICEs under -mslow-flash-data and -mword-relocations because there

> > >>>> is no way to load an address, both literal pools and MOVW/MOVT being

> > >>>> forbidden. This patch gives an error message when both options are

> > >>>> specified by the user and adds the according dg-skip-if directives for

> > >>>> tests that use either of these options.

> > >>>>

> > >>>> ChangeLog entries are as follows:

> > >>>>

> > >>>> *** gcc/ChangeLog ***

> > >>>>

> > >>>> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

> > >>>>

> > >>>>        PR target/87374

> > >>>>        * config/arm/arm.c (arm_option_check_internal): Disable the combined

> > >>>>        use of -mslow-flash-data and -mword-relocations.

> > >>>>

> > >>>> *** gcc/testsuite/ChangeLog ***

> > >>>>

> > >>>> 2018-09-25  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

> > >>>>

> > >>>>        PR target/87374

> > >>>>        * gcc.target/arm/movdi_movt.c: Skip if both -mslow-flash-data and

> > >>>>        -mword-relocations would be passed when compiling the test.

> > >>>>        * gcc.target/arm/movsi_movt.c: Likewise.

> > >>>>        * gcc.target/arm/pr81863.c: Likewise.

> > >>>>        * gcc.target/arm/thumb2-slow-flash-data-1.c: Likewise.

> > >>>>        * gcc.target/arm/thumb2-slow-flash-data-2.c: Likewise.

> > >>>>        * gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.

> > >>>>        * gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.

> > >>>>        * gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.

> > >>>>        * gcc.target/arm/tls-disable-literal-pool.c: Likewise.

> > >>>>

> > >>>>

> > >>>> Testing: Bootstrapped in Thumb-2 mode. No testsuite regression when

> > >>>> targeting arm-none-eabi. Modified tests get skipped as expected when

> > >>>> running the testsuite with -mslow-flash-data (pr81863.c) or

> > >>>> -mword-relocations (all the others).

> > >>>>

> > >>>>

> > >>>> Is this ok for trunk? I'd also appreciate guidance on whether this is

> > >>>> worth a backport. It's a simple patch but on the other hand it only

> > >>>> prevents some option combination, it does not fix anything so I have

> > >>>> mixed feelings.

> > >>>

> > >>> In my opinion -mslow-flash-data is more of a tuning option rather than a security/ABI feature

> > >>> and therefore erroring out on its combination with -mword-relocations feels odd.

> > >>> I'm leaning more towards making -mword-relocations or any other option that really requires constant pools

> > >>> to bypass/disable the effects of -mslow-flash-data instead.

> > >>

> > >> -mslow-flash-data and -mword-relocations are contradictory in their

> > >> expectations. mslow-flash-data is for not putting anything in the

> > >> literal pool whereas mword-relocations is purely around the use of movw

> > >> / movt instructions for word sized values. I wish we had called

> > >> -mslow-flash-data something else (probably -mno-literal-pools).

> > >> -mslow-flash-data is used primarily by M-profile users and

> > >> -mword-relocations IIUC was a point fix for use in the Linux kernel for

> > >> module loads at a time when not all module loaders in the linux kernel

> > >> were fixed for the movw / movt relocations and armv7-a / thumb2 was in

> > >> it's infancy :). Thus they are used by different constituencies in

> > >> general and I wouldn't see them used together by actual users.

> > >

> > > Technically, -mslow-flash-data does not forbid literal pool, it just

> > > discourages it because it's slower than many instructions. -mpure-code

> > > on the other hand reuse the same logic and does forbid literal pools.

> > > We could treat -mslow-flash-data differently but the question is

> > > whether it is worth the trouble.

> >

> > Well, yeah I don't see the need for it. You could argue that

> > -mslow-flash-data can be porous but realistically if you want this as an

> > effective performance option , such porosity should be discouraged very

> > strongly ;)

> >

> > >

> > > By the way, I've noticed that the documentation for -mword-relocations

> > > says it defaults to on for -fpic and -fPIC but when looking through

> > > the code I saw that target_word_relocation is not set in these case,

> > > rather the initial commit checks that introduced -mword-relocation

> > > also checks for flag_pic when checking target_word_relocation. However

> > > a later commit added one more check for target_word_relocations but

> > > nothing for flag_pic. I'm now consolidating this so that flag_pic sets

> > > target_word_relocations. I'll do a regression testing with -fPIC and

> > > then post an updated patch.

> >

> > I'm reasonably sure that's *not* going to have *any* effect on code

> > generation as in the -fpic / -fPIC case we always produce the funny GOT

> > unspecs and have never used movw / movt instructions in those sequences

> > for addressing. If that had happened most of the world's dynamic

> > libraries would have faulted by now because I don't think they can

> > process absolute movw / movt relocations.

> >

> >

> > It is automatically implied by the fact that we never produced PC

> > relative versions of the immediates that get put into movw / movt

> > instructions. I don't even remember us having effective relocations to

> > implement this but this is going back a few years now.

> >

> >

> > Sure that probably needs a comment rather than being implicit from the

> > source or from my own head :)

> >

> > Ramana

Patch

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 6332e68df05..5beffc875c1 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -2893,17 +2893,22 @@  arm_option_check_internal (struct gcc_options *opts)
       flag_pic = 0;
     }
 
-  /* We only support -mpure-code and -mslow-flash-data on M-profile targets
-     with MOVT.  */
-  if ((target_pure_code || target_slow_flash_data)
-      && (!TARGET_HAVE_MOVT || arm_arch_notm || flag_pic || TARGET_NEON))
+  if (target_pure_code || target_slow_flash_data)
     {
       const char *flag = (target_pure_code ? "-mpure-code" :
 					     "-mslow-flash-data");
-      error ("%s only supports non-pic code on M-profile targets with the "
-	     "MOVT instruction", flag);
-    }
 
+      /* We only support -mpure-code and -mslow-flash-data on M-profile targets
+	 with MOVT.  */
+      if (!TARGET_HAVE_MOVT || arm_arch_notm || flag_pic || TARGET_NEON)
+	error ("%s only supports non-pic code on M-profile targets with the "
+	       "MOVT instruction", flag);
+
+      /* Cannot load addresses: -mslow-flash-data forbids literal pool and
+	 -mword-relocations forbids relocation of MOVT/MOVW.  */
+      if (target_word_relocations)
+	error ("%s incompatible with -mword-relocations", flag);
+    }
 }
 
 /* Recompute the global settings depending on target attribute options.  */
diff --git a/gcc/testsuite/gcc.target/arm/movdi_movt.c b/gcc/testsuite/gcc.target/arm/movdi_movt.c
index e2a28ccbd99..a01ffa0dc93 100644
--- a/gcc/testsuite/gcc.target/arm/movdi_movt.c
+++ b/gcc/testsuite/gcc.target/arm/movdi_movt.c
@@ -1,4 +1,5 @@ 
 /* { dg-do compile { target { arm_cortex_m && { arm_thumb2_ok || arm_thumb1_movt_ok } } } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-O2 -mslow-flash-data" } */
 
 unsigned long long
diff --git a/gcc/testsuite/gcc.target/arm/movsi_movt.c b/gcc/testsuite/gcc.target/arm/movsi_movt.c
index 3cf46e2fd17..19d202ecd33 100644
--- a/gcc/testsuite/gcc.target/arm/movsi_movt.c
+++ b/gcc/testsuite/gcc.target/arm/movsi_movt.c
@@ -1,4 +1,5 @@ 
 /* { dg-do compile { target { arm_cortex_m && { arm_thumb2_ok || arm_thumb1_movt_ok } } } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-O2 -mslow-flash-data" } */
 
 unsigned
diff --git a/gcc/testsuite/gcc.target/arm/pr81863.c b/gcc/testsuite/gcc.target/arm/pr81863.c
index 63b1ed66b2c..225a0c5cc2b 100644
--- a/gcc/testsuite/gcc.target/arm/pr81863.c
+++ b/gcc/testsuite/gcc.target/arm/pr81863.c
@@ -1,5 +1,6 @@ 
 /* testsuite/gcc.target/arm/pr48183.c */
 /* { dg-do compile } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mslow-flash-data" } } */
 /* { dg-options "-O2 -mword-relocations -march=armv7-a -marm" } */
 /* { dg-final { scan-assembler-not "\[\\t \]+movw" } } */
 
diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c
index 089a72b67f3..d10391a69ac 100644
--- a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c
+++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-1.c
@@ -6,6 +6,7 @@ 
 /* { dg-do compile } */
 /* { dg-require-effective-target arm_cortex_m } */
 /* { dg-require-effective-target arm_thumb2_ok } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-O2 -mthumb -mslow-flash-data" } */
 
 float sf;
diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c
index c87e050639d..90bd44e27e5 100644
--- a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c
+++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-2.c
@@ -3,6 +3,7 @@ 
 /* { dg-require-effective-target arm_thumb2_ok } */
 /* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */
 /* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-march=armv7e-m+fp -mfloat-abi=hard -O2 -mthumb -mslow-flash-data" } */
 
 float f (float);
diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c
index 8c6210ee6c9..5d9cd9c4df2 100644
--- a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c
+++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-3.c
@@ -3,6 +3,7 @@ 
 /* { dg-require-effective-target arm_thumb2_ok } */
 /* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */
 /* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-march=armv7e-m+fp -mfloat-abi=hard -mthumb -mslow-flash-data" } */
 
 /* From PR71607 */
diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c
index 1bcb6924ed2..0eeddd5e6ec 100644
--- a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c
+++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-4.c
@@ -3,6 +3,7 @@ 
 /* { dg-require-effective-target arm_thumb2_ok } */
 /* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */
 /* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-march=armv7e-m+fp -mfloat-abi=hard -O2 -mthumb -mslow-flash-data" } */
 
 double __attribute__ ((target ("fpu=fpv5-d16")))
diff --git a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c
index 808fff05faa..7d52f3801b6 100644
--- a/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c
+++ b/gcc/testsuite/gcc.target/arm/thumb2-slow-flash-data-5.c
@@ -3,6 +3,7 @@ 
 /* { dg-require-effective-target arm_thumb2_ok } */
 /* { dg-skip-if "avoid conflicts with multilib options" { *-*-* } { "-mcpu=*" } { "-mcpu=cortex-m4" "-mcpu=cortex-m7" } } */
 /* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=hard" } } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-march=armv7e-m+fp -mfloat-abi=hard -O2 -mthumb -mslow-flash-data" } */
 
 double __attribute__ ((target ("fpu=fpv5-sp-d16")))
diff --git a/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c b/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c
index 283201fdd97..834eaf6db92 100644
--- a/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c
+++ b/gcc/testsuite/gcc.target/arm/tls-disable-literal-pool.c
@@ -2,6 +2,7 @@ 
 /* { dg-require-effective-target tls_native } */
 /* { dg-require-effective-target arm_cortex_m } */
 /* { dg-require-effective-target arm_thumb2_ok } */
+/* { dg-skip-if "-mslow-flash-data and -mword-relocations incompatible" { *-*-* } { "-mword-relocations" } } */
 /* { dg-options "-mslow-flash-data" } */
 
 __thread int x = 0;