* ARM: Emit conditions in push_multi
@ 2011-09-01 11:53 Bernd Schmidt
2011-09-02 10:35 ` Ramana Radhakrishnan
0 siblings, 1 reply; 7+ messages in thread
From: Bernd Schmidt @ 2011-09-01 11:53 UTC (permalink / raw)
To: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 226 bytes --]
Shrink-wrapping tests on ARM had one additional failure, which I could
track down to a stmfd instruction being emitted where an stmhifd was
intended. The following patch fixes the testcase; full tests running
now. Ok?
Bernd
[-- Attachment #2: stmcond.diff --]
[-- Type: text/plain, Size: 771 bytes --]
* config/arm/arm.md (push_multi): Emit predicates.
Index: gcc/config/arm/arm.md
===================================================================
--- gcc/config/arm/arm.md (revision 178135)
+++ gcc/config/arm/arm.md (working copy)
@@ -10581,14 +10581,14 @@ (define_insn "*push_multi"
In Thumb mode always use push, and the assembler will pick
something appropriate. */
if (num_saves == 1 && TARGET_ARM)
- output_asm_insn (\"str\\t%1, [%m0, #-4]!\", operands);
+ output_asm_insn (\"str%?\\t%1, [%m0, #-4]!\", operands);
else
{
int i;
char pattern[100];
if (TARGET_ARM)
- strcpy (pattern, \"stmfd\\t%m0!, {%1\");
+ strcpy (pattern, \"stm%(fd%)\\t%m0!, {%1\");
else
strcpy (pattern, \"push\\t{%1\");
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: ARM: Emit conditions in push_multi 2011-09-01 11:53 ARM: Emit conditions in push_multi Bernd Schmidt @ 2011-09-02 10:35 ` Ramana Radhakrishnan 2011-09-02 11:44 ` Bernd Schmidt 0 siblings, 1 reply; 7+ messages in thread From: Ramana Radhakrishnan @ 2011-09-02 10:35 UTC (permalink / raw) To: Bernd Schmidt; +Cc: GCC Patches On 1 September 2011 12:50, Bernd Schmidt <bernds@codesourcery.com> wrote: > Shrink-wrapping tests on ARM had one additional failure, which I could > track down to a stmfd instruction being emitted where an stmhifd was > intended. The following patch fixes the testcase; full tests running > now. Ok? IIUC this should have been a result of conditionalizing the prologue saves by the CCFSM state machine in ARM state given that the push instruction below doesn't have the conditional markers. In which case the routines to emit the asm for the VFP registers( vfp_output_fstmfd? ) should also be checked for this issue. cheers Ramana ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ARM: Emit conditions in push_multi 2011-09-02 10:35 ` Ramana Radhakrishnan @ 2011-09-02 11:44 ` Bernd Schmidt 2011-09-07 9:12 ` Ramana Radhakrishnan 0 siblings, 1 reply; 7+ messages in thread From: Bernd Schmidt @ 2011-09-02 11:44 UTC (permalink / raw) To: Ramana Radhakrishnan; +Cc: GCC Patches [-- Attachment #1: Type: text/plain, Size: 935 bytes --] On 09/02/11 12:35, Ramana Radhakrishnan wrote: > On 1 September 2011 12:50, Bernd Schmidt <bernds@codesourcery.com> wrote: >> Shrink-wrapping tests on ARM had one additional failure, which I could >> track down to a stmfd instruction being emitted where an stmhifd was >> intended. The following patch fixes the testcase; full tests running >> now. Ok? > > IIUC this should have been a result of conditionalizing the prologue > saves by the CCFSM state machine in ARM state Correct. > given that the push > instruction below doesn't have the conditional markers. Although I'm not sure how you arrived at this? Thumb insns can't be conditional anyway? > In which case > the routines to emit the asm for the VFP registers( vfp_output_fstmfd? > ) should also be checked for this issue. Hmm, ok. I found two more places which looked suspicious. New version, untested so far. What's "sfmfd"? That doesn't occur in my manual. Bernd [-- Attachment #2: arm-multi.diff --] [-- Type: text/plain, Size: 1589 bytes --] * config/arm/arm.md (push_multi): Emit predicates. (push_fp_multi): Likewise. * config/arm/arm.c (vfp_output_fstmd): Likewise. Index: gcc/config/arm/arm.c =================================================================== --- gcc/config/arm/arm.c (revision 178135) +++ gcc/config/arm/arm.c (working copy) @@ -13084,7 +13096,7 @@ vfp_output_fstmd (rtx * operands) int base; int i; - strcpy (pattern, "fstmfdd\t%m0!, {%P1"); + strcpy (pattern, "fstmfdd%?\t%m0!, {%P1"); p = strlen (pattern); gcc_assert (GET_CODE (operands[1]) == REG); Index: gcc/config/arm/arm.md =================================================================== --- gcc/config/arm/arm.md (revision 178135) +++ gcc/config/arm/arm.md (working copy) @@ -10581,14 +10581,14 @@ (define_insn "*push_multi" In Thumb mode always use push, and the assembler will pick something appropriate. */ if (num_saves == 1 && TARGET_ARM) - output_asm_insn (\"str\\t%1, [%m0, #-4]!\", operands); + output_asm_insn (\"str%?\\t%1, [%m0, #-4]!\", operands); else { int i; char pattern[100]; if (TARGET_ARM) - strcpy (pattern, \"stmfd\\t%m0!, {%1\"); + strcpy (pattern, \"stm%(fd%)\\t%m0!, {%1\"); else strcpy (pattern, \"push\\t{%1\"); @@ -10631,7 +10631,7 @@ (define_insn "*push_fp_multi" { char pattern[100]; - sprintf (pattern, \"sfmfd\\t%%1, %d, [%%m0]!\", XVECLEN (operands[2], 0)); + sprintf (pattern, \"sfm%(fd%)\\t%%1, %d, [%%m0]!\", XVECLEN (operands[2], 0)); output_asm_insn (pattern, operands); return \"\"; }" ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ARM: Emit conditions in push_multi 2011-09-02 11:44 ` Bernd Schmidt @ 2011-09-07 9:12 ` Ramana Radhakrishnan 2011-09-07 19:36 ` Bernd Schmidt 0 siblings, 1 reply; 7+ messages in thread From: Ramana Radhakrishnan @ 2011-09-07 9:12 UTC (permalink / raw) To: Bernd Schmidt; +Cc: GCC Patches On 2 September 2011 12:42, Bernd Schmidt <bernds@codesourcery.com> wrote: > On 09/02/11 12:35, Ramana Radhakrishnan wrote: >> On 1 September 2011 12:50, Bernd Schmidt <bernds@codesourcery.com> wrote: >>> Shrink-wrapping tests on ARM had one additional failure, which I could >>> track down to a stmfd instruction being emitted where an stmhifd was >>> intended. The following patch fixes the testcase; full tests running >>> now. Ok? >> >> IIUC this should have been a result of conditionalizing the prologue >> saves by the CCFSM state machine in ARM state > > Correct. > >> given that the push >> instruction below doesn't have the conditional markers. > > Although I'm not sure how you arrived at this? Thumb insns can't be > conditional anyway? IT blocks in Thumb2 and the predicable attribute means we can generate some amount of conditional instruction in T2 state. I just looked at that patch as it was and didn't remember whether the pattern had a predicable attribute set on it ( in which case you would have seen the failure in Thumb2). Thus if the failure came in ARM state alone it had to be from the CCFSM state machine. It's probably worth-while just putting out the %? marker in that case anyway for the push instruction given that the only way this is likely to be seen is if someday we mark this as predicable :) > >> In which case >> the routines to emit the asm for the VFP registers( vfp_output_fstmfd? >> ) should also be checked for this issue. > > Hmm, ok. I found two more places which looked suspicious. New version, > untested so far. What's "sfmfd"? That doesn't occur in my manual. sfmfd an FPA instruction should be conditionalizable given it's in the co-processor space. cheers Ramana ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ARM: Emit conditions in push_multi 2011-09-07 9:12 ` Ramana Radhakrishnan @ 2011-09-07 19:36 ` Bernd Schmidt 2011-09-08 8:20 ` Ramana Radhakrishnan 2011-09-12 11:41 ` Jakub Jelinek 0 siblings, 2 replies; 7+ messages in thread From: Bernd Schmidt @ 2011-09-07 19:36 UTC (permalink / raw) To: Ramana Radhakrishnan; +Cc: GCC Patches [-- Attachment #1: Type: text/plain, Size: 1175 bytes --] On 09/07/11 10:28, Ramana Radhakrishnan wrote: > On 2 September 2011 12:42, Bernd Schmidt <bernds@codesourcery.com> wrote: >> On 09/02/11 12:35, Ramana Radhakrishnan wrote: >>> On 1 September 2011 12:50, Bernd Schmidt <bernds@codesourcery.com> wrote: >>>> Shrink-wrapping tests on ARM had one additional failure, which I could >>>> track down to a stmfd instruction being emitted where an stmhifd was >>>> intended. The following patch fixes the testcase; full tests running >>>> now. Ok? >>> >>> IIUC this should have been a result of conditionalizing the prologue >>> saves by the CCFSM state machine in ARM state >> >> Correct. >> >>> given that the push >>> instruction below doesn't have the conditional markers. >> >> Although I'm not sure how you arrived at this? Thumb insns can't be >> conditional anyway? > > IT blocks in Thumb2 and the predicable attribute means we can generate > some amount of conditional instruction in T2 state. Oh, and the insns are output as conditional in addition to the it instruction - half a year of hardly doing anything with ARM and I'd forgotten about this :( New patch below. Tested on arm-eabi sim with a few multilibs. Bernd [-- Attachment #2: arm-multi2.diff --] [-- Type: text/plain, Size: 1658 bytes --] * config/arm/arm.md (push_multi): Emit predicates. (push_fp_multi): Likewise. * config/arm/arm.c (vfp_output_fstmd): Likewise. Index: gcc/config/arm/arm.c =================================================================== --- gcc/config/arm/arm.c (revision 178596) +++ gcc/config/arm/arm.c (working copy) @@ -13084,7 +13084,7 @@ vfp_output_fstmd (rtx * operands) int base; int i; - strcpy (pattern, "fstmfdd\t%m0!, {%P1"); + strcpy (pattern, "fstmfdd%?\t%m0!, {%P1"); p = strlen (pattern); gcc_assert (GET_CODE (operands[1]) == REG); Index: gcc/config/arm/arm.md =================================================================== --- gcc/config/arm/arm.md (revision 178596) +++ gcc/config/arm/arm.md (working copy) @@ -10581,14 +10581,16 @@ (define_insn "*push_multi" In Thumb mode always use push, and the assembler will pick something appropriate. */ if (num_saves == 1 && TARGET_ARM) - output_asm_insn (\"str\\t%1, [%m0, #-4]!\", operands); + output_asm_insn (\"str%?\\t%1, [%m0, #-4]!\", operands); else { int i; char pattern[100]; if (TARGET_ARM) - strcpy (pattern, \"stmfd\\t%m0!, {%1\"); + strcpy (pattern, \"stm%(fd%)\\t%m0!, {%1\"); + else if (TARGET_THUMB2) + strcpy (pattern, \"push%?\\t{%1\"); else strcpy (pattern, \"push\\t{%1\"); @@ -10631,7 +10633,7 @@ (define_insn "*push_fp_multi" { char pattern[100]; - sprintf (pattern, \"sfmfd\\t%%1, %d, [%%m0]!\", XVECLEN (operands[2], 0)); + sprintf (pattern, \"sfm%(fd%)\\t%%1, %d, [%%m0]!\", XVECLEN (operands[2], 0)); output_asm_insn (pattern, operands); return \"\"; }" ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ARM: Emit conditions in push_multi 2011-09-07 19:36 ` Bernd Schmidt @ 2011-09-08 8:20 ` Ramana Radhakrishnan 2011-09-12 11:41 ` Jakub Jelinek 1 sibling, 0 replies; 7+ messages in thread From: Ramana Radhakrishnan @ 2011-09-08 8:20 UTC (permalink / raw) To: Bernd Schmidt; +Cc: GCC Patches > New patch below. Tested on arm-eabi sim with a few multilibs. OK. Ramana ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: ARM: Emit conditions in push_multi 2011-09-07 19:36 ` Bernd Schmidt 2011-09-08 8:20 ` Ramana Radhakrishnan @ 2011-09-12 11:41 ` Jakub Jelinek 1 sibling, 0 replies; 7+ messages in thread From: Jakub Jelinek @ 2011-09-12 11:41 UTC (permalink / raw) To: Bernd Schmidt; +Cc: Ramana Radhakrishnan, GCC Patches Hi! This last hunk is wrong, for sprintf you need to use %% instead of %. On Wed, Sep 07, 2011 at 09:23:36PM +0200, Bernd Schmidt wrote: > @@ -10631,7 +10633,7 @@ (define_insn "*push_fp_multi" > { > char pattern[100]; > > - sprintf (pattern, \"sfmfd\\t%%1, %d, [%%m0]!\", XVECLEN (operands[2], 0)); > + sprintf (pattern, \"sfm%(fd%)\\t%%1, %d, [%%m0]!\", XVECLEN (operands[2], 0)); > output_asm_insn (pattern, operands); > return \"\"; > }" Committed as obvious: 2011-09-12 Jakub Jelinek <jakub@redhat.com> PR bootstrap/50352 * config/arm/arm.md (*push_fp_multi): Add % before %( and %) in the sprintf format string. --- gcc/config/arm/arm.md (revision 178777) +++ gcc/config/arm/arm.md (revision 178778) @@ -10633,7 +10633,7 @@ (define_insn "*push_fp_multi" { char pattern[100]; - sprintf (pattern, \"sfm%(fd%)\\t%%1, %d, [%%m0]!\", XVECLEN (operands[2], 0)); + sprintf (pattern, \"sfm%%(fd%%)\\t%%1, %d, [%%m0]!\", XVECLEN (operands[2], 0)); output_asm_insn (pattern, operands); return \"\"; }" Jakub ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-09-12 9:37 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-09-01 11:53 ARM: Emit conditions in push_multi Bernd Schmidt 2011-09-02 10:35 ` Ramana Radhakrishnan 2011-09-02 11:44 ` Bernd Schmidt 2011-09-07 9:12 ` Ramana Radhakrishnan 2011-09-07 19:36 ` Bernd Schmidt 2011-09-08 8:20 ` Ramana Radhakrishnan 2011-09-12 11:41 ` Jakub Jelinek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).