On 02/10/2018 11:42, Thomas Preudhomme wrote:
Hi Ramana,
On Thu, 27 Sep 2018 at 11:14, Ramana Radhakrishnan
<ramana.radhakrish...@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.preudho...@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.preudho...@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
