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.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.
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
