[PATCH/RFC/RFA] Machine modes for address printing (all targets)
Hi, Depending on assembler syntax and supported addressing modes, several targets need to know the machine mode for a memory access when printing an address (i.e. for automodify addresses that need to know the size of their access), but it is not available with the current TARGET_PRINT_OPERAND_ADDRESS hook. This leads to an ugly corner in the operand output mechanism, where address printing gets split between different parts of a backend, or some other hack (e.g. a global variable) is used to communicate the machine mode to the address printing hook. Using a global variable also leads to a latent (?) bug on at least AArch64: attempts to use the 'a' operand printing code cause final.c to call output_address (in turn invoking the PRINT_OPERAND_ADDRESS macro) *without* first setting the magic global aarch64_memory_reference_mode, which means a stale value will be used instead. The full list of targets that use some form of workaround for the lack of machine mode in the address printing hook is (E&OE): aarch64: uses magic global. arc: pre/post inc/dec handled in print_operand. arm: uses magic global. c6x epiphany: offsets handled in print_operand. m32r: hard-wires 4 for access size. nds32 tilegx: uses magic global. tilepro: uses magic global. That's not all targets by any means, but may be enough to warrant a change in the interface. I propose that: * The output_address function should have a machine_mode argument added. Bare addresses (e.g. the 'a' case in final.c) should pass "VOIDmode" for this argument. * Other callers of output_address -- actually all in backends -- can pass the machine mode for the memory access in question. * The TARGET_PRINT_OPERAND_ADDRESS hook shall also have a machine_mode argument added. The legacy PRINT_OPERAND_ADDRESS hook can be left alone. (The documentation for the operand-printing hooks needs fixing too, incidentally.) The attached patch makes this change, fairly mechanically. This removes (most of) the magic globals for address printing, but I haven't tried to refactor the targets that use other hacks to print correct auto-modify addresses (that can be done by their respective maintainers, hopefully, and should result in a nice cleanup). Unfortunately I can't hope to test all the targets affected, though the subset of targets that it's relatively easy for me to build, build fine. I also ran regression tests for AArch64. OK to apply, or any comments, or any further testing required? Thanks, Julian ChangeLog gcc/ * final.c (output_asm_insn): Pass VOIDmode to output_address. (output_address): Add MODE argument. Pass to print_operand_address hook. * targhooks.c (default_print_operand_address): Add MODE argument. * targhooks.h (default_print_operand_address): Update prototype. * output.h (output_address): Update prototype. * target.def (print_operand_address): Add MODE argument. * config/vax/vax.c (print_operand_address): Pass VOIDmode to output_address. (print_operand): Pass access mode to output_address. * config/mcore/mcore.c (mcore_print_operand_address): Add MODE argument. (mcore_print_operand): Update calls to mcore_print_operand_address. * config/fr30/fr30.c (fr30_print_operand): Pass VOIDmode to output_address. * config/lm32/lm32.c (lm32_print_operand): Pass mode in calls to output_address. * config/tilegx/tilegx.c (output_memory_reference_mode): Remove global. (tilegx_print_operand): Don't set above global. Update calls to output_address. (tilegx_print_operand_address): Add MODE argument. Use instead of output_memory_reference_mode global. * config/frv/frv.c (frv_print_operand_address): Add MODE argument. * config/mn10300/mn10300.c (mn10300_print_operand): Pass mode to output_address. * config/cris/cris.c (cris_print_operand_address): Add MODE argument. (cris_print_operand): Pass mode to output_address calls. * config/spu/spu.c (print_operand): Pass mode to output_address calls. * config/aarch64/aarch64.h (aarch64_print_operand) (aarch64_print_operand_address): Remove prototypes. * config/aarch64/aarch64.c (aarch64_memory_reference_mode): Delete global. (aarch64_print_operand): Make static. Update calls to output_address. (aarch64_print_operand_address): Add MODE argument. Use instead of aarch64_memory_reference_mode global. (TARGET_PRINT_OPERAND, TARGET_PRINT_OPERAND_ADDRESS): Define target hooks. * config/aarch64/aarch64.h (PRINT_OPERAND, PRINT_OPERAND_ADDRESS): Delete macro definitions. * config/pa/pa.c (pa_print_operand): Pass mode in output_address calls. * config/xtensa/xtensa.c (print_operand): Pass mode in output_address calls. * config/h8300/h8300.c (h8300_print_operand_address): Add MODE argument. (h83000_print_operand): Update calls to h8300_print_operand_address and output_address. * config/ia64/ia64.c (ia64_print_operand_ad
Re: [PATCH/RFC/RFA] Machine modes for address printing (all targets)
On 11/04/2015 01:54 PM, Julian Brown wrote:
That's not all targets by any means, but may be enough to warrant a
change in the interface. I propose that:
* The output_address function should have a machine_mode argument
added. Bare addresses (e.g. the 'a' case in final.c) should pass
"VOIDmode" for this argument.
* Other callers of output_address -- actually all in backends -- can
pass the machine mode for the memory access in question.
* The TARGET_PRINT_OPERAND_ADDRESS hook shall also have a machine_mode
argument added. The legacy PRINT_OPERAND_ADDRESS hook can be left
alone. (The documentation for the operand-printing hooks needs fixing
too, incidentally.)
I think this approach seems fine.
static void
-mcore_print_operand_address (FILE * stream, rtx x)
+mcore_print_operand_address (FILE * stream, machine_mode mode ATTRIBUTE_UNUSED,
+rtx x)
So apparently we're settling on writing the unused arg as just
"machine_mode" without a name. Please change everywhere.
@@ -1754,7 +1754,7 @@ mmix_print_operand_punct_valid_p (unsign
/* TARGET_PRINT_OPERAND_ADDRESS. */
static void
-mmix_print_operand_address (FILE *stream, rtx x)
+mmix_print_operand_address (FILE *stream, machine_mode mode, rtx x)
{
if (REG_P (x))
{
The arg appears to be unused - I'd expect to see a warning here.
Other thank that it looks OK. I'm not going to require that you test
every target, but it would be good to have the full set built to cc1
before and after, and please be on the lookout for fallout.
Bernd
Re: [PATCH/RFC/RFA] Machine modes for address printing (all targets)
On Thu, 5 Nov 2015 11:22:04 +0100
Bernd Schmidt wrote:
> > static void
> > -mcore_print_operand_address (FILE * stream, rtx x)
> > +mcore_print_operand_address (FILE * stream, machine_mode mode
> > ATTRIBUTE_UNUSED,
> > +rtx x)
>
> So apparently we're settling on writing the unused arg as just
> "machine_mode" without a name. Please change everywhere.
>
> > @@ -1754,7 +1754,7 @@ mmix_print_operand_punct_valid_p (unsign
> > /* TARGET_PRINT_OPERAND_ADDRESS. */
> >
> > static void
> > -mmix_print_operand_address (FILE *stream, rtx x)
> > +mmix_print_operand_address (FILE *stream, machine_mode mode, rtx x)
> > {
> >if (REG_P (x))
> > {
>
> The arg appears to be unused - I'd expect to see a warning here.
I've fixed those two, and a handful of other bits I missed.
> Other thank that it looks OK. I'm not going to require that you test
> every target, but it would be good to have the full set built to cc1
> before and after, and please be on the lookout for fallout.
Thanks! I used the attached "build-all.sh" to test all the targets
affected by the patch with "make all-gcc": those now all succeed
(I'm sure I reinvented a wheel here, but perhaps the target list is
useful to someone else).
Julian
ChangeLog
gcc/
* final.c (output_asm_insn): Pass VOIDmode to output_address.
(output_address): Add MODE argument. Pass to print_operand_address
hook.
* targhooks.c (default_print_operand_address): Add MODE argument.
* targhooks.h (default_print_operand_address): Update prototype.
* output.h (output_address): Update prototype.
* target.def (print_operand_address): Add MODE argument.
* config/vax/vax.c (print_operand_address): Pass VOIDmode to
output_address.
(print_operand): Pass access mode to output_address.
* config/mcore/mcore.c (mcore_print_operand_address): Add MODE
argument.
(mcore_print_operand): Update calls to mcore_print_operand_address.
* config/fr30/fr30.c (fr30_print_operand): Pass VOIDmode to
output_address.
* config/lm32/lm32.c (lm32_print_operand): Pass mode in calls to
output_address.
* config/tilegx/tilegx.c (output_memory_reference_mode): Remove
global.
(tilegx_print_operand): Don't set above global. Update calls to
output_address.
(tilegx_print_operand_address): Add MODE argument. Use instead of
output_memory_reference_mode global.
* config/frv/frv.c (frv_print_operand_address): Add MODE argument.
(frv_print_operand): Pass mode to frv_print_operand_address calls.
* config/mn10300/mn10300.c (mn10300_print_operand): Pass mode to
output_address.
* config/cris/cris.c (cris_print_operand_address): Add MODE
argument.
(cris_print_operand): Pass mode to output_address calls.
* config/spu/spu.c (print_operand): Pass mode to output_address
calls.
* config/aarch64/aarch64.h (aarch64_print_operand)
(aarch64_print_operand_address): Remove prototypes.
* config/aarch64/aarch64.c (aarch64_memory_reference_mode): Delete
global.
(aarch64_print_operand): Make static. Update calls to
output_address.
(aarch64_print_operand_address): Add MODE argument. Use instead of
aarch64_memory_reference_mode global.
(TARGET_PRINT_OPERAND, TARGET_PRINT_OPERAND_ADDRESS): Define target
hooks.
* config/aarch64/aarch64.h (PRINT_OPERAND, PRINT_OPERAND_ADDRESS):
Delete macro definitions.
* config/pa/pa.c (pa_print_operand): Pass mode in output_address
calls.
* config/xtensa/xtensa.c (print_operand): Pass mode in
output_address calls.
* config/h8300/h8300.c (h8300_print_operand_address): Add MODE
argument.
(h83000_print_operand): Update calls to h8300_print_operand_address
and output_address.
* config/ia64/ia64.c (ia64_print_operand_address): Add MODE
argument.
* config/tilepro/tilepro.c (output_memory_reference_mode): Delete
global.
(tilepro_print_operand): Pass mode to output_address.
(tilepro_print_operand_address): Add MODE argument. Use instead of
output_memory_reference_mode.
* config/nvptx/nvptx.c (output_decl_chunk, nvptx_assemble_integer)
(nvptx_output_call_insn, nvptx_print_address_operand): Pass VOIDmode
to output_address calls.
(nvptx_print_operand_address): Add MODE argument.
* config/alpha/alpha.c (print_operand): Pass mode argument in
output_address calls.
* config/m68k/m68k.c (print_operand): Pass mode argument in
output_address call.
* config/avr/avr.c (avr_print_operand_address): Add MODE argument.
(avr_print_operand): Update calls to avr_print_operand_address.
* config/sparc/sparc.c (sparc_print_operand_address): Add MODE
argument. Update calls to output_address.
(sparc_print_operand): Pass mode to output_address.
* config/iq2000/iq2000.c (iq2000_print_operand_address): Add MODE
argument.
(iq2000_print_operand): Pass mode in output_address calls.
* config/stormy16/stormy
Re: [PATCH/RFC/RFA] Machine modes for address printing (all targets)
On Mon, 9 Nov 2015, Julian Brown wrote: > Thanks! I used the attached "build-all.sh" to test all the targets > affected by the patch with "make all-gcc": those now all succeed > (I'm sure I reinvented a wheel here, but perhaps the target list is > useful to someone else). The wheel you reinvented is called contrib/config-list.mk (which (a) requires you to have a native bootstrapped compiler from current trunk in your PATH - it uses --enable-werror-always so that cross compilers fail for warnings that would cause a native compiler bootstrap to fail - and (b) is intended to build compilers for targets covering all significantly different variations, though I think you could use it with a custom target list). -- Joseph S. Myers jos...@codesourcery.com
