[Bug target/49468] SH Target: inefficient integer abs code

2013-12-05 Thread olegendo at gcc dot gnu.org
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468

Oleg Endo  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #11 from Oleg Endo  ---
I think this can be closed.


[Bug target/49468] SH Target: inefficient integer abs code

2012-03-14 Thread olegendo at gcc dot gnu.org
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468

--- Comment #10 from Oleg Endo  2012-03-14 
22:58:05 UTC ---
Author: olegendo
Date: Wed Mar 14 22:57:58 2012
New Revision: 185397

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=185397
Log:
PR target/49468
* gcc.target/sh/pr49468-si.c: Make dg-skip-if not the first directive.
* gcc.target/sh/pr49468-di.c: Likewise.


Modified:
trunk/gcc/testsuite/ChangeLog
trunk/gcc/testsuite/gcc.target/sh/pr49468-di.c
trunk/gcc/testsuite/gcc.target/sh/pr49468-si.c


[Bug target/49468] SH Target: inefficient integer abs code

2012-02-29 Thread kkojima at gcc dot gnu.org
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468

--- Comment #9 from Kazumoto Kojima  2012-02-29 
23:18:23 UTC ---
(In reply to comment #8)
Perhaps.  Anyway looks fine to me except one minor failure
on sh64-elf:

xsh64-elf-combined/combined/libgcc/libgcc2.c: In function '__powisf2':
xsh64-elf-combined/combined/libgcc/libgcc2.c:1779:1: error: unrecognizable
insn:
(insn 11 10 12 3 (set (reg:DI 170)
(abs:DI (reg:DI 169)))
xsh64-elf-combined/combined/libgcc/libgcc2.c:1770 -1
 (nil))
xsh64-elf-combined/combined/libgcc/libgcc2.c:1779:1: internal compiler error:
in extract_insn, at recog.c:2123

The failure went away if restricting new absdi2 expander to TARGET_SH1.

--- gcc/config/sh/sh.md~2012-02-29 10:52:16.0 +0900
+++ gcc/config/sh/sh.md2012-02-29 11:07:42.0 +0900
@@ -4538,7 +4538,7 @@ label:
   [(set (match_operand:DI 0 "arith_reg_dest" "")
 (abs:DI (match_operand:DI 1 "arith_reg_operand" "")))
(clobber (reg:SI T_REG))]
-  ""
+  "TARGET_SH1"
   "")

 (define_insn_and_split "*absdi2"


[Bug target/49468] SH Target: inefficient integer abs code

2012-02-28 Thread olegendo at gcc dot gnu.org
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468

--- Comment #8 from Oleg Endo  2012-02-28 08:41:28 
UTC ---
Created attachment 26768
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26768
Patch to add DImode abs

The attached patch adds DImode abs and did not introduce new failures when
tested against rev 184589.
I'm a bit confused, as this is exactly the same code as I used before to add
DImode abs, and before it was failing in some cases.  Either there was another
bug that got fixed in the middle-end optimizers, or this is just coincidence. 
I've seen that other expanders use force_reg to make sure that operands will be
in regs ... would that be the safer way of doing it?


[Bug target/49468] SH Target: inefficient integer abs code

2012-02-26 Thread olegendo at gcc dot gnu.org
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468

olegendo at gcc dot gnu.org changed:

   What|Removed |Added

 Status|UNCONFIRMED |ASSIGNED
   Last reconfirmed||2012-02-26
 CC||olegendo at gcc dot gnu.org
 AssignedTo|unassigned at gcc dot   |olegendo at gcc dot gnu.org
   |gnu.org |
 Ever Confirmed|0   |1


[Bug target/49468] SH Target: inefficient integer abs code

2011-09-25 Thread oleg.e...@t-online.de
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468

Oleg Endo  changed:

   What|Removed |Added

  Attachment #24625|0   |1
is obsolete||

--- Comment #7 from Oleg Endo  2011-09-25 12:48:24 UTC 
---
Created attachment 25360
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25360
Proposed patch

The last version of the patch fails the test
gcc.c-torture/execute/arith-rand-ll.c for -m2a-single -mb and multiple
optimization levels with the following error:

internal compiler error: in change_address_1, at emit-rtl.c:1994


The attached version fixes some of the failures but still fails the test above
with -m2a-single -mb -O2. Other optimization levels work fine.

The problem is caused by the define_insn_and_split "*abssi2". It even fails if
the "*abssi2" splits into nothing but a simple register copy (movdi) or
comparison insn. 

I'm now testing the patch without the DI abs parts and will submit it if passes
without new failures.

Cheers,
Oleg


[Bug target/49468] SH Target: inefficient integer abs code

2011-06-28 Thread oleg.e...@t-online.de
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468

Oleg Endo  changed:

   What|Removed |Added

  Attachment #24603|0   |1
is obsolete||

--- Comment #6 from Oleg Endo  2011-06-29 01:36:46 UTC 
---
Created attachment 24625
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24625
Proposed patch

Added missing constraints to define_insn_and_split.
Fixed the formatting (hopefully ... there seems to be a problem with the tabs.
I'm using 4 spaces wide tabs)


[Bug target/49468] SH Target: inefficient integer abs code

2011-06-26 Thread kkojima at gcc dot gnu.org
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468

--- Comment #5 from Kazumoto Kojima  2011-06-27 
06:39:40 UTC ---
Argh, I also missed clobbers.  Looks fine to me now, except
that insn_and_split "*negdi2" forgot to set constraints and
some minor coding style issues below.

The first comment should be started with a capital letter and
ended with a period.  Also please follow GCC C coding style
even for C program segments in .md file.  C lines in the patch
are started with a tab instead of 2 spaces.  A long conditional
should be broken like as

  (cond
   ? value0
   : value1)

instead of

  (cond ?
 value0 :
 value1)

Please use braces

"
{
  int low_word = ...
  ...

  emit_insn (...
  DONE;
}")

instead of

"
  int low_word = ...
  ...

  emit_insn (...
  DONE;
")

especially when new variables are used, though those braces
aren't required with the current gen* tools.

> + emit_insn (gen_negsi_cond (operands[0], operands[1], operands[1], 
> + GEN_INT (1)));

The first line has an extra space after the last comma and
the indentation of the 2nd line doesn't match with GCC coding
standard.  BTW, you could use const[01]_rtx for GEN_INT ([01]):

  emit_insn (gen_negsi_cond (operands[0], operands[1], operands[1],
 const1_rtx));

There are similar extra white space + broken indentation issues:

> +(define_insn_and_split "negsi_cond"
> +  [(set (match_operand:SI 0 "arith_reg_dest" "=r,r")
> + (if_then_else:SI (eq:SI (reg:SI T_REG) 
> + (match_operand:SI 3 
> "const_int_operand" "M,N"))
...
> +   emit_label_after (skip_neg_label, 
> + emit_insn (gen_negsi2 
> (operands[0], operands[1])));
...

Perhaps mail or editor problem?


[Bug target/49468] SH Target: inefficient integer abs code

2011-06-26 Thread oleg.e...@t-online.de
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468

Oleg Endo  changed:

   What|Removed |Added

  Attachment #24560|0   |1
is obsolete||

--- Comment #4 from Oleg Endo  2011-06-26 20:48:19 UTC 
---
Created attachment 24603
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24603
Proposed patch

Thanks for checking.
I missed to tell the expanders that the T bit is clobbered by insns such as
negdi2 and abssi2 / absdi2. The negdi2 expander that I have changed caused the
libstdc++ tests to fail when formatting a -1LL.

Another (reduced) example:


long long x (long long i, int j, long long k)
{
if (j & 5)
return -i;

return -k;
}

..ended up as (the "j & 5" got lost)...

mov.l@(4,r15),r1
clrt
mov.l@r15,r2
negcr1,r1
negcr2,r0
bt/s.L8
clrt
negcr5,r1
negcr4,r0
.L8:
rts
nop


[Bug target/49468] SH Target: inefficient integer abs code

2011-06-22 Thread kkojima at gcc dot gnu.org
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468

Kazumoto Kojima  changed:

   What|Removed |Added

   Severity|normal  |enhancement

--- Comment #3 from Kazumoto Kojima  2011-06-22 
22:37:28 UTC ---
On sh4-unknown-linux-gnu, this patch causes two new failures on
libstdc++ testsuite

FAIL: 27_io/basic_ostream/inserters_arithmetic/char/7.cc execution test
FAIL: 27_io/basic_ostream/inserters_arithmetic/wchar_t/7.cc execution test

I can't find any differences between generated codes for those
test cases by compilers with/without your patch and the failures
go away if the tests are running with libstdc++ library built
with the unpatched compiler.
So it seems that something in libstdc++ library is miscompiled.
Weired and hard to see what is going on, ATM.


[Bug target/49468] SH Target: inefficient integer abs code

2011-06-19 Thread oleg.e...@t-online.de
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468

--- Comment #2 from Oleg Endo  2011-06-19 15:48:41 UTC 
---
Created attachment 24561
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24561
Before/After Examples


[Bug target/49468] SH Target: inefficient integer abs code

2011-06-19 Thread oleg.e...@t-online.de
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49468

--- Comment #1 from Oleg Endo  2011-06-19 15:29:45 UTC 
---
Created attachment 24560
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24560
Proposed patch

The patch adds explicit handling of abs:SI and abs:DI to the machine
description instead of relying on the default abs handling. On SH4 the
zero-offset branch way is definitely faster. It reduces pressure on EX group
instructions and improves parallel instruction execution. On other than SH4
zero-offset branches are not as fast, but the resulting code should still be
faster than the default branch-free abs code, at least it is more compact. 
The patch also handles the case of neg (abs (...)) by simply inverting the
branch condition.

Feedback appreciated.