diff mbox

rs6000: Use xori for HTM builtins and vector compares

Message ID 3970eebb20386837608385f3e276f7c33080e8c0.1410289982.git.segher@kernel.crashing.org
State New
Headers show

Commit Message

Segher Boessenkool Sept. 9, 2014, 7:29 p.m. UTC 
These patterns use subfic now.  subfic clobbers the carry.  Other code
already preferably uses xor.  Let's do the same here.

Bootstrapped and tested as usual, no regressions.  Is this okay to apply?


Segher


2014-09-09  Segher Boessenkool  <segher@kernel.crashing.org>

	* config/rs6000/htm.md (tabort, tabortdc, tabortdci, tabortwc,
	tabortwci, tbegin, tcheck, tend, trechkpt, treclaim, tsr): Use xor
	instead of minus.
	* config/rs6000/vector.md (cr6_test_for_zero_reverse,
	cr6_test_for_lt_reverse): Ditto.


---
 gcc/config/rs6000/htm.md    | 33 ++++++++++++++++++++++-----------
 gcc/config/rs6000/vector.md |  8 ++++++--
 2 files changed, 28 insertions(+), 13 deletions(-)

Comments

David Edelsohn Sept. 9, 2014, 11:28 p.m. UTC | #1 
On Tue, Sep 9, 2014 at 3:29 PM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> These patterns use subfic now.  subfic clobbers the carry.  Other code
> already preferably uses xor.  Let's do the same here.
>
> Bootstrapped and tested as usual, no regressions.  Is this okay to apply?
>
>
> Segher
>
>
> 2014-09-09  Segher Boessenkool  <segher@kernel.crashing.org>
>
>         * config/rs6000/htm.md (tabort, tabortdc, tabortdci, tabortwc,
>         tabortwci, tbegin, tcheck, tend, trechkpt, treclaim, tsr): Use xor
>         instead of minus.
>         * config/rs6000/vector.md (cr6_test_for_zero_reverse,
>         cr6_test_for_lt_reverse): Ditto.

This is okay with me, but let me give Peter a chance to comment if
there was a specific reason to use subfic instead of xori. This may
have been a carry-over from Z, which does not have the same CA clobber
issue.

Thanks, David
Peter Bergner Sept. 10, 2014, 6:38 p.m. UTC | #2 
On Tue, 2014-09-09 at 19:28 -0400, David Edelsohn wrote:
> On Tue, Sep 9, 2014 at 3:29 PM, Segher Boessenkool
> > 2014-09-09  Segher Boessenkool  <segher@kernel.crashing.org>
> >
> >         * config/rs6000/htm.md (tabort, tabortdc, tabortdci, tabortwc,
> >         tabortwci, tbegin, tcheck, tend, trechkpt, treclaim, tsr): Use xor
> >         instead of minus.
> >         * config/rs6000/vector.md (cr6_test_for_zero_reverse,
> >         cr6_test_for_lt_reverse): Ditto.
> 
> This is okay with me, but let me give Peter a chance to comment if
> there was a specific reason to use subfic instead of xori. This may
> have been a carry-over from Z, which does not have the same CA clobber
> issue.

Actually, I just copied the usage in cr6_test_for_zero_reverse and
cr6_test_for_lt_reverse, so I'm not against using xori...as long as
compiling a "if __builtin_tbegin (0) {...}" still ends up with a
.tbegin followed immediately by a branch (ie, no interleaving copy
from CR and compare instruction).

Peter
Segher Boessenkool Sept. 10, 2014, 7:29 p.m. UTC | #3 
On Wed, Sep 10, 2014 at 01:38:13PM -0500, Peter Bergner wrote:
> On Tue, 2014-09-09 at 19:28 -0400, David Edelsohn wrote:
> > On Tue, Sep 9, 2014 at 3:29 PM, Segher Boessenkool
> > > 2014-09-09  Segher Boessenkool  <segher@kernel.crashing.org>
> > >
> > >         * config/rs6000/htm.md (tabort, tabortdc, tabortdci, tabortwc,
> > >         tabortwci, tbegin, tcheck, tend, trechkpt, treclaim, tsr): Use xor
> > >         instead of minus.
> > >         * config/rs6000/vector.md (cr6_test_for_zero_reverse,
> > >         cr6_test_for_lt_reverse): Ditto.
> > 
> > This is okay with me, but let me give Peter a chance to comment if
> > there was a specific reason to use subfic instead of xori. This may
> > have been a carry-over from Z, which does not have the same CA clobber
> > issue.
> 
> Actually, I just copied the usage in cr6_test_for_zero_reverse and
> cr6_test_for_lt_reverse, so I'm not against using xori...as long as
> compiling a "if __builtin_tbegin (0) {...}" still ends up with a
> .tbegin followed immediately by a branch (ie, no interleaving copy
> from CR and compare instruction).

Huh, interesting.  I assumed 1-(0_or_1) and (0_or_1)^1 would look the
same to combine, but no.

With subfic, combine optimises it all to a branch on cr0.  With xori,
for some reason combine has a much easier job, and it optimises the lot
to a copy of cr0 to some cc, and then branch on that.  The RA of course
gets rid of the copy.  The extra freedom will more likely help than hurt.

The simple testcase ends up as just "tbegin. 0; beqlr 0" in either case.

So, okay?


Segher
Peter Bergner Sept. 10, 2014, 8:20 p.m. UTC | #4 
On Wed, 2014-09-10 at 14:29 -0500, Segher Boessenkool wrote:
> Huh, interesting.  I assumed 1-(0_or_1) and (0_or_1)^1 would look the
> same to combine, but no.
> 
> With subfic, combine optimises it all to a branch on cr0.  With xori,
> for some reason combine has a much easier job, and it optimises the lot
> to a copy of cr0 to some cc, and then branch on that.  The RA of course
> gets rid of the copy.  The extra freedom will more likely help than hurt.
> 
> The simple testcase ends up as just "tbegin. 0; beqlr 0" in either case.
> 
> So, okay?

No objections from me then.

Peter
diff mbox

Patch

diff --git a/gcc/config/rs6000/htm.md b/gcc/config/rs6000/htm.md
index 140212b..03948b1 100644
--- a/gcc/config/rs6000/htm.md
+++ b/gcc/config/rs6000/htm.md
@@ -55,7 +55,8 @@  (define_expand "tabort"
 	(eq:SI (match_dup 2)
 	       (const_int 0)))
    (set (match_operand:SI 0 "int_reg_operand" "")
-	(minus:SI (const_int 1) (match_dup 3)))]
+	(xor:SI (match_dup 3)
+		(const_int 1)))]
   "TARGET_HTM"
 {
   operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
@@ -81,7 +82,8 @@  (define_expand "tabortdc"
 	(eq:SI (match_dup 4)
 	       (const_int 0)))
    (set (match_operand:SI 0 "int_reg_operand" "")
-	(minus:SI (const_int 1) (match_dup 5)))]
+	(xor:SI (match_dup 5)
+		(const_int 1)))]
   "TARGET_HTM"
 {
   operands[4] = gen_rtx_REG (CCmode, CR0_REGNO);
@@ -109,7 +111,8 @@  (define_expand "tabortdci"
 	(eq:SI (match_dup 4)
 	       (const_int 0)))
    (set (match_operand:SI 0 "int_reg_operand" "")
-	(minus:SI (const_int 1) (match_dup 5)))]
+	(xor:SI (match_dup 5)
+		(const_int 1)))]
   "TARGET_HTM"
 {
   operands[4] = gen_rtx_REG (CCmode, CR0_REGNO);
@@ -137,7 +140,8 @@  (define_expand "tabortwc"
 	(eq:SI (match_dup 4)
 	       (const_int 0)))
    (set (match_operand:SI 0 "int_reg_operand" "")
-	(minus:SI (const_int 1) (match_dup 5)))]
+	(xor:SI (match_dup 5)
+		(const_int 1)))]
   "TARGET_HTM"
 {
   operands[4] = gen_rtx_REG (CCmode, CR0_REGNO);
@@ -165,7 +169,8 @@  (define_expand "tabortwci"
 	(eq:SI (match_dup 4)
 	       (const_int 0)))
    (set (match_operand:SI 0 "int_reg_operand" "")
-	(minus:SI (const_int 1) (match_dup 5)))]
+	(xor:SI (match_dup 5)
+		(const_int 1)))]
   "TARGET_HTM"
 {
   operands[4] = gen_rtx_REG (CCmode, CR0_REGNO);
@@ -209,7 +214,8 @@  (define_expand "tbegin"
 	(eq:SI (match_dup 2)
 	       (const_int 0)))
    (set (match_operand:SI 0 "int_reg_operand" "")
-	(minus:SI (const_int 1) (match_dup 3)))]
+	(xor:SI (match_dup 3)
+		(const_int 1)))]
   "TARGET_HTM"
 {
   operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
@@ -233,7 +239,8 @@  (define_expand "tcheck"
 	(eq:SI (match_dup 2)
 	       (const_int 0)))
    (set (match_operand:SI 0 "int_reg_operand" "")
-	(minus:SI (const_int 1) (match_dup 3)))]
+	(xor:SI (match_dup 3)
+		(const_int 1)))]
   "TARGET_HTM"
 {
   operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
@@ -257,7 +264,8 @@  (define_expand "tend"
 	(eq:SI (match_dup 2)
 	       (const_int 0)))
    (set (match_operand:SI 0 "int_reg_operand" "")
-	(minus:SI (const_int 1) (match_dup 3)))]
+	(xor:SI (match_dup 3)
+		(const_int 1)))]
   "TARGET_HTM"
 {
   operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
@@ -281,7 +289,8 @@  (define_expand "trechkpt"
 	(eq:SI (match_dup 1)
 	       (const_int 0)))
    (set (match_operand:SI 0 "int_reg_operand" "")
-	(minus:SI (const_int 1) (match_dup 2)))]
+	(xor:SI (match_dup 2)
+		(const_int 1)))]
   "TARGET_HTM"
 {
   operands[1] = gen_rtx_REG (CCmode, CR0_REGNO);
@@ -305,7 +314,8 @@  (define_expand "treclaim"
 	(eq:SI (match_dup 2)
 	       (const_int 0)))
    (set (match_operand:SI 0 "int_reg_operand" "")
-	(minus:SI (const_int 1) (match_dup 3)))]
+	(xor:SI (match_dup 3)
+		(const_int 1)))]
   "TARGET_HTM"
 {
   operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
@@ -329,7 +339,8 @@  (define_expand "tsr"
 	(eq:SI (match_dup 2)
 	       (const_int 0)))
    (set (match_operand:SI 0 "int_reg_operand" "")
-	(minus:SI (const_int 1) (match_dup 3)))]
+	(xor:SI (match_dup 3)
+		(const_int 1)))]
   "TARGET_HTM"
 {
   operands[2] = gen_rtx_REG (CCmode, CR0_REGNO);
diff --git a/gcc/config/rs6000/vector.md b/gcc/config/rs6000/vector.md
index bfae244..237724e 100644
--- a/gcc/config/rs6000/vector.md
+++ b/gcc/config/rs6000/vector.md
@@ -686,7 +686,9 @@  (define_expand "cr6_test_for_zero_reverse"
   [(set (match_operand:SI 0 "register_operand" "=r")
 	(eq:SI (reg:CC 74)
 	       (const_int 0)))
-   (set (match_dup 0) (minus:SI (const_int 1) (match_dup 0)))]
+   (set (match_dup 0)
+	(xor:SI (match_dup 0)
+		(const_int 1)))]
   "TARGET_ALTIVEC || TARGET_VSX"
   "")
 
@@ -701,7 +703,9 @@  (define_expand "cr6_test_for_lt_reverse"
   [(set (match_operand:SI 0 "register_operand" "=r")
 	(lt:SI (reg:CC 74)
 	       (const_int 0)))
-   (set (match_dup 0) (minus:SI (const_int 1) (match_dup 0)))]
+   (set (match_dup 0)
+	(xor:SI (match_dup 0)
+		(const_int 1)))]
   "TARGET_ALTIVEC || TARGET_VSX"
   "")