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