diff mbox

RFC: [PATCH,ARM] Fix 56110

Message ID 20130218214741.GA17018@code-monkey.de
State New
Headers show

Commit Message

Tilman Sauerbeck Feb. 18, 2013, 9:47 p.m. UTC 
Hi,
adding the instruction pattern below fixes my testcase for PR 56110;
however I'm not sure if adding a new pattern is the correct way to go.

I duplicated the andsi3_compare0_scratch pattern, and lifted the
requirement that the 2nd operand be an arm_not_operand. I didn't copy
over the clobber pattern because I don't know what it does ;)

Comments? Can anyone put me in the right direction here? Or take my
humble attempt and massage it into the proper fix?
I can provide a dejagnu testcase if it helps.

No ChangeLog entry because I know this diff won't go anywhere in its
current incarnation.

Thanks,
Tilman

Comments

Tilman Sauerbeck Feb. 18, 2013, 9:52 p.m. UTC | #1 
Tilman Sauerbeck [2013-02-18 22:47]:

> [...]
> +  "TARGET_32BIT"
> +  "@
> +   tst%?\\t%0, %1"
> [...]

I managed to post the wrong diff -- line 2 in the citation should be
omitted of course.  Sorry.

Regards,
Tilman
Richard Earnshaw Feb. 19, 2013, 3:12 p.m. UTC | #2 
On 18/02/13 21:47, Tilman Sauerbeck wrote:
> Hi,
> adding the instruction pattern below fixes my testcase for PR 56110;
> however I'm not sure if adding a new pattern is the correct way to go.
>
> I duplicated the andsi3_compare0_scratch pattern, and lifted the
> requirement that the 2nd operand be an arm_not_operand. I didn't copy
> over the clobber pattern because I don't know what it does ;)
>
> Comments? Can anyone put me in the right direction here? Or take my
> humble attempt and massage it into the proper fix?
> I can provide a dejagnu testcase if it helps.
>
> No ChangeLog entry because I know this diff won't go anywhere in its
> current incarnation.
>
> Thanks,
> Tilman
>
> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
> index 64888f9..e47f8f7 100644
> --- a/gcc/config/arm/arm.md
> +++ b/gcc/config/arm/arm.md
> @@ -2212,6 +2212,19 @@
>      (set_attr "type"  "simple_alu_imm,simple_alu_imm,*")]
>   )
>
> +(define_insn "*andsi3_compare0_scratch2"
> +  [(set (reg:CC_NOOV CC_REGNUM)
> +	(compare:CC_NOOV
> +	 (and:SI (match_operand:SI 0 "s_register_operand" "r,r,r")
> +		 (match_operand:SI 1 "const_int_operand" "r,r,r"))
> +	 (const_int 0)))]
> +  "TARGET_32BIT"
> +  "@
> +   tst%?\\t%0, %1"
> +  [(set_attr "conds" "set")
> +   (set_attr "type"  "simple_alu_imm,simple_alu_imm,*")]
> +)
> +
>   (define_insn "*zeroextractsi_compare0_scratch"
>     [(set (reg:CC_NOOV CC_REGNUM)
>   	(compare:CC_NOOV (zero_extract:SI
>



Sorry, this is not correct.  You've got a constraint that requires a 
const_int_operand, but then specified only a register.  This will force 
the compiler to reload the immediate operand into a register, just 
negating any saving you've made from getting rid of the compare 
instruction.  Then you've added three identical variants all using two 
registers of the same class.

However, the question you need to be asking is why the pattern 
immediately before the one you've added is not matching.  The compiler 
knows how to add clobbers, so I'm surprised that you're finding a new 
pattern to be necessary.

R.
diff mbox

Patch

diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 64888f9..e47f8f7 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -2212,6 +2212,19 @@ 
    (set_attr "type"  "simple_alu_imm,simple_alu_imm,*")]
 )
 
+(define_insn "*andsi3_compare0_scratch2"
+  [(set (reg:CC_NOOV CC_REGNUM)
+	(compare:CC_NOOV
+	 (and:SI (match_operand:SI 0 "s_register_operand" "r,r,r")
+		 (match_operand:SI 1 "const_int_operand" "r,r,r"))
+	 (const_int 0)))]
+  "TARGET_32BIT"
+  "@
+   tst%?\\t%0, %1"
+  [(set_attr "conds" "set")
+   (set_attr "type"  "simple_alu_imm,simple_alu_imm,*")]
+)
+
 (define_insn "*zeroextractsi_compare0_scratch"
   [(set (reg:CC_NOOV CC_REGNUM)
 	(compare:CC_NOOV (zero_extract:SI