Patchwork ARM: Emit conditions in push_multi

login
register
mail settings
Submitter Bernd Schmidt
Date Sept. 2, 2011, 11:42 a.m.
Message ID <4E60C110.4030407@codesourcery.com>
Download mbox | patch
Permalink /patch/113129/
State New
Headers show

Comments

Bernd Schmidt - Sept. 2, 2011, 11:42 a.m.
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
* config/arm/arm.md (push_multi): Emit predicates.
	(push_fp_multi): Likewise.
	* config/arm/arm.c (vfp_output_fstmd): Likewise.
Ramana Radhakrishnan - Sept. 7, 2011, 8:28 a.m.
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

Patch

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 \"\";
   }"