Ping.
https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00395.html
Thanks,
Kyrill
On 05/05/16 12:50, Kyrill Tkachov wrote:
Hi all,
In this PR we deal with some fallout from the conversion to unified assembly.
We now end up emitting instructions like:
pop {r0,r1,r2,r3,pc}^
which is not legal. We have to use an LDM form.
There are bugs in two arm.c functions: output_return_instruction and
arm_output_multireg_pop.
In output_return_instruction the buggy hunk from the conversion was:
else
- if (TARGET_UNIFIED_ASM)
sprintf (instr, "pop%s\t{", conditional);
- else
- sprintf (instr, "ldm%sfd\t%%|sp!, {", conditional);
The code was already very obscurely structured and arguably the bug was latent.
It emitted POP only when TARGET_UNIFIED_ASM was on, and since
TARGET_UNIFIED_ASM was on
only for Thumb, we never went down this path interrupt handling code, since the
interrupt
attribute is only available for ARM code. After the removal of
TARGET_UNIFIED_ASM we ended up
using POP unconditionally. So this patch adds a check for IS_INTERRUPT and
outputs the
appropriate LDM form.
In arm_output_multireg_pop the buggy hunk was:
- if ((regno_base == SP_REGNUM) && TARGET_THUMB)
+ if ((regno_base == SP_REGNUM) && update)
{
- /* Output pop (not stmfd) because it has a shorter encoding. */
- gcc_assert (update);
sprintf (pattern, "pop%s\t{", conditional);
}
Again, the POP was guarded on TARGET_THUMB and so would never be taken on
interrupt handling
routines. This patch guards that with the appropriate check on interrupt return.
Also, there are a couple of bugs in the 'else' branch of that 'if':
* The "ldmfd%s" was output without a '\t' at the end which meant that the base
register
name would be concatenated with the 'ldmfd', creating invalid assembly.
* The logic:
if (regno_base == SP_REGNUM)
/* update is never true here, hence there is no need to handle
pop here. */
sprintf (pattern, "ldmfd%s", conditional);
if (update)
sprintf (pattern, "ldmia%s\t", conditional);
else
sprintf (pattern, "ldm%s\t", conditional);
Meant that for "regno == SP_REGNUM && !update" we'd end up printing
"ldmfd%sldm%s\t"
to pattern. I didn't manage to reproduce that condition though, so maybe it
can't ever occur.
This patch fixes both these issues nevertheless.
I've added the testcase from the PR to catch the fix in
output_return_instruction.
The testcase doesn't catch the bugs in arm_output_multireg_pop, but the
existing tests
gcc.target/arm/interrupt-1.c and gcc.target/arm/interrupt-2.c would have caught
them
if only they were assemble tests rather than just compile. So this patch makes
them
assembly tests (and reverts the scan-assembler checks for the correct LDM
pattern).
Bootstrapped and tested on arm-none-linux-gnueabihf.
Ok for trunk and GCC 6?
Thanks,
Kyrill
2016-05-05 Kyrylo Tkachov <kyrylo.tkac...@arm.com>
PR target/70830
* config/arm/arm.c (arm_output_multireg_pop): Avoid POP instruction
when popping the PC and within an interrupt handler routine.
Add missing tab to output of "ldmfd".
(output_return_instruction): Output LDMFD with SP update rather
than POP when returning from interrupt handler.
2016-05-05 Kyrylo Tkachov <kyrylo.tkac...@arm.com>
PR target/70830
* gcc.target/arm/interrupt-1.c: Change dg-compile to dg-assemble.
Add -save-temps to dg-options.
Scan for ldmfd rather than pop instruction.
* gcc.target/arm/interrupt-2.c: Likewise.
* gcc.target/arm/pr70830.c: New test.
