On 28/09/2023 14:29, Richard Ball wrote:
Follow up patch to arm: Use deltas for Arm switch tables
This patch moves the switch tables for Arm from the .text section
into the .rodata section.
gcc/ChangeLog:
* config/arm/aout.h: Change to use the Lrtx label.
* config/arm/arm.h (CASE_VECTOR_PC_RELATIVE): Remove arm targets
from (!target_pure_code) condition.
(ADDR_VEC_ALIGN): Add align for tables in rodata section.
* config/arm/arm.cc (arm_output_casesi): Alter the function to include
.Lrtx label and remove adr instructions.
* config/arm/arm.md
(arm_casesi_internal): Use force_reg to generate ldr instructions that
would otherwise be out of range, and change rtl to accommodate force
reg.
Additionally remove unnecessary register temp.
(casesi): Remove pure code check for Arm.
* config/arm/elf.h (JUMP_TABLES_IN_TEXT_SECTION): Remove arm
targets from JUMP_TABLES_IN_TEXT_SECTION definition.
gcc/testsuite/ChangeLog:
* gcc.target/arm/arm-switchstatement.c: Alter the tests to
change adr instruction to ldr.
This all looks pretty good, but there are some minor niggles to sort out
before it can go in...
arm.cc:
arm_output_casesi (rtx *operands)
{
+ char buf[100];
buf is unused, so this breaks a native bootstrap.
output_asm_insn ("add\t%|pc, %|pc, %4, lsl #2", operands);;
Two semicolons at the end of the line.
+ else
+ {
+ output_asm_insn ("ldr\t%|pc, [%5, %0, lsl #2]", operands);
+ }
Our normal coding style is to omit the braces for a single statement in
an 'if/else' clause, even if the other arm of the clause uses braces, so:
else
output_asm_insn ("ldr\t%|pc, [%5, %0, lsl #2]", operands);
+ output_asm_insn ("nop;", operands);
Stray semicolon after the "nop".
#define CASE_VECTOR_PC_RELATIVE (TARGET_ARM || ((TARGET_THUMB2 \
|| (TARGET_THUMB1 \
&& (optimize_size || flag_pic))) \
&& (!target_pure_code)))
The indentation here is incorrect, which makes it very hard to
understand the logic. But I think a bit of reordering would help
clarify things as well..
#define CASE_VECTOR_PC_RELATIVE
(TARGET_ARM
|| (!target_pure_code
&& (TARGET_THUMB2
|| (TARGET_THUMB1 && (optimize_size || flag_pic)))))
(obviously with the line escapes added back in)
arm.md (casesi):
"TARGET_ARM || ((TARGET_THUMB2 || optimize_size || flag_pic) &&
^^
operators should be at the start of the following line, not the end of
the previous one.
(!target_pure_code))"
So:
"TARGET_ARM || ((TARGET_THUMB2 || optimize_size || flag_pic)
&& (!target_pure_code))"
But I think this could be laid out better as well:
"(TARGET_ARM
|| (!target_pure_code
&& (TARGET_THUMB2 || optimize_size || flag_pic)))"
arm_casesi_internal:
rtx tmp = force_reg (SImode, gen_rtx_LABEL_REF (SImode, operands[2]));
Tmp is not generally a good choice of name, even for short fragments
like this. Use something more descriptive to the object it holds, like
"lref"; or, better still, a name that describes what the label points to
(vec_table_ref?).
elf.h:
/* We put Thumb-2 jump tables in the text section, because it makes
the code more efficient, but for Thumb-1 and ARM it's better to put
them out of
band unless we are generating compressed tables. */
This comment is misleading now, as it implies that compressed tables for
arm are still sometimes placed in the text segment (the unless clause)
and that's not true.
R.
