* [Patch : H8300] Bug fix for bit insn and minor tweaks to insns
@ 2011-06-09 7:40 Kaushik Phatak
2011-06-10 18:37 ` Jeff Law
2011-06-13 13:16 ` Eric Botcazou
0 siblings, 2 replies; 5+ messages in thread
From: Kaushik Phatak @ 2011-06-09 7:40 UTC (permalink / raw)
To: gcc-patches; +Cc: law, Prafulla Thakare
[-- Attachment #1: Type: text/plain, Size: 856 bytes --]
Hi,
The following patch fixes an ICE that is generated when the compiler tries
to perform bit manipulation for logical operations when the source and
destination address does not match. The testcase is also included in the
patch(gcc.dg).
The additional condition in the insn takes care of the ICE which occurs at '-O1'.
The other insn's are reordered to give preference to bit instructions using existing
constraints.
Ok to apply?
Thanks & Regards,
Kaushik Phatak
www.kpitgnutools.com
2011-06-09 Kaushik Phatak <kaushik.phatak@kpitcummins.com>
* config/h8300/h8300.md (bsetqi_msx, bclrqi_msx, bnotqi_msx): Added
condition to disallow non-identical memory locations.
(*andqi3_2, andqi3_1, iorqi3_1, xorqi3_1): Reorder insn to give
preference to bit manipulation instructions.
* gcc.dg/h8300-bit-insn-ice2.2: New testcase.
[-- Attachment #2: h8_bit.diff --]
[-- Type: application/octet-stream, Size: 5423 bytes --]
Index: gcc/config/h8300/h8300.md
===================================================================
--- gcc/config/h8300/h8300.md (revision 174833)
+++ gcc/config/h8300/h8300.md (working copy)
@@ -1767,7 +1767,8 @@
[(set (match_operand:QI 0 "bit_register_indirect_operand" "=WU")
(and:QI (match_operand:QI 1 "bit_register_indirect_operand" "%0")
(match_operand:QI 2 "single_zero_operand" "Y0")))]
- "TARGET_H8300SX"
+ "TARGET_H8300SX
+ && INTVAL (XEXP (operands[0], 0)) == INTVAL (XEXP (operands[1], 0))"
"bclr\\t%W2,%0"
[(set_attr "length" "8")])
@@ -1800,29 +1801,31 @@
"TARGET_H8300SX"
"bclr\\t%W2,%0"
[(set_attr "length" "8")])
+
(define_insn "*andqi3_2"
- [(set (match_operand:QI 0 "bit_operand" "=rQ,r")
- (and:QI (match_operand:QI 1 "bit_operand" "%0,WU")
- (match_operand:QI 2 "h8300_src_operand" "rQi,IP1>X")))]
+ [(set (match_operand:QI 0 "bit_operand" "=U,rQ,r")
+ (and:QI (match_operand:QI 1 "bit_operand" "%0,0,WU")
+ (match_operand:QI 2 "h8300_src_operand" "Y0,rQi,IP1>X")))]
"TARGET_H8300SX"
"@
- and %X2,%X0
- bfld %2,%1,%R0"
- [(set_attr "length" "*,8")
- (set_attr "length_table" "logicb,*")
- (set_attr "cc" "set_znv,none_0hit")])
+ bclr\\t %W2,%R0
+ and %X2,%X0
+ bfld %2,%1,%R0"
+ [(set_attr "length" "8,*,8")
+ (set_attr "length_table" "*,logicb,*")
+ (set_attr "cc" "none_0hit,set_znv,none_0hit")])
(define_insn "andqi3_1"
- [(set (match_operand:QI 0 "bit_operand" "=r,U")
+ [(set (match_operand:QI 0 "bit_operand" "=U,r")
(and:QI (match_operand:QI 1 "bit_operand" "%0,0")
- (match_operand:QI 2 "h8300_src_operand" "rn,n")))]
+ (match_operand:QI 2 "h8300_src_operand" "Y0,rn")))]
"register_operand (operands[0], QImode)
|| single_zero_operand (operands[2], QImode)"
"@
- and %X2,%X0
- bclr %W2,%R0"
+ bclr %W2,%R0
+ and %X2,%X0"
[(set_attr "length" "2,8")
- (set_attr "cc" "set_znv,none_0hit")])
+ (set_attr "cc" "none_0hit,set_znv")])
(define_expand "andqi3"
[(set (match_operand:QI 0 "register_operand" "")
@@ -1903,11 +1906,13 @@
;; ----------------------------------------------------------------------
;; OR INSTRUCTIONS
;; ----------------------------------------------------------------------
+
(define_insn "bsetqi_msx"
[(set (match_operand:QI 0 "bit_register_indirect_operand" "=WU")
(ior:QI (match_operand:QI 1 "bit_register_indirect_operand" "%0")
(match_operand:QI 2 "single_one_operand" "Y2")))]
- "TARGET_H8300SX"
+ "TARGET_H8300SX
+ && INTVAL (XEXP (operands[0], 0)) == INTVAL (XEXP (operands[1], 0))"
"bset\\t%V2,%0"
[(set_attr "length" "8")])
@@ -1942,18 +1947,19 @@
[(set_attr "length" "8")])
(define_insn "iorqi3_1"
- [(set (match_operand:QI 0 "bit_operand" "=rQ,U")
+ [(set (match_operand:QI 0 "bit_operand" "=U,rQ")
(ior:QI (match_operand:QI 1 "bit_operand" "%0,0")
- (match_operand:QI 2 "h8300_src_operand" "rQi,n")))]
+ (match_operand:QI 2 "h8300_src_operand" "Y2,rQi")))]
"TARGET_H8300SX || register_operand (operands[0], QImode)
|| single_one_operand (operands[2], QImode)"
"@
- or\\t%X2,%X0
- bset\\t%V2,%R0"
- [(set_attr "length" "*,8")
- (set_attr "length_table" "logicb,*")
- (set_attr "cc" "set_znv,none_0hit")])
+ bset\\t%V2,%R0
+ or\\t%X2,%X0"
+ [(set_attr "length" "8,*")
+ (set_attr "length_table" "*,logicb")
+ (set_attr "cc" "none_0hit,set_znv")])
+
(define_expand "iorqi3"
[(set (match_operand:QI 0 "register_operand" "")
(ior:QI (match_operand:QI 1 "register_operand" "")
@@ -1982,7 +1988,8 @@
[(set (match_operand:QI 0 "bit_register_indirect_operand" "=WU")
(xor:QI (match_operand:QI 1 "bit_register_indirect_operand" "%0")
(match_operand:QI 2 "single_one_operand" "Y2")))]
- "TARGET_H8300SX"
+ "TARGET_H8300SX
+ && INTVAL (XEXP (operands[0], 0)) == INTVAL (XEXP (operands[1], 0))"
"bnot\\t%V2,%0"
[(set_attr "length" "8")])
@@ -2017,18 +2024,19 @@
[(set_attr "length" "8")])
(define_insn "xorqi3_1"
- [(set (match_operand:QI 0 "bit_operand" "=r,U")
+ [(set (match_operand:QI 0 "bit_operand" "=U,r")
(xor:QI (match_operand:QI 1 "bit_operand" "%0,0")
- (match_operand:QI 2 "h8300_src_operand" "rQi,n")))]
+ (match_operand:QI 2 "h8300_src_operand" "Y2,rQi")))]
"TARGET_H8300SX || register_operand (operands[0], QImode)
|| single_one_operand (operands[2], QImode)"
"@
- xor\\t%X2,%X0
- bnot\\t%V2,%R0"
- [(set_attr "length" "*,8")
- (set_attr "length_table" "logicb,*")
- (set_attr "cc" "set_znv,none_0hit")])
+ bnot\\t%V2,%R0
+ xor\\t%X2,%X0"
+ [(set_attr "length" "8,*")
+ (set_attr "length_table" "*,logicb")
+ (set_attr "cc" "none_0hit,set_znv")])
+
(define_expand "xorqi3"
[(set (match_operand:QI 0 "register_operand" "")
(xor:QI (match_operand:QI 1 "register_operand" "")
Index: gcc/testsuite/gcc.dg/h8300-bit-insn-ice2.c
===================================================================
--- gcc/testsuite/gcc.dg/h8300-bit-insn-ice2.c (revision 0)
+++ gcc/testsuite/gcc.dg/h8300-bit-insn-ice2.c (revision 0)
@@ -0,0 +1,15 @@
+/* { dg-skip-if "" { "h8300*-*-*" } "*" "-msx*" } */
+/* { dg-options "-O2" } */
+/* ICE for bit instruction generation using 16-bit const */
+
+#define MSTPCRA (*(volatile unsigned char*)0xFFFFC9)
+#define MSTPCRA2 (*(volatile unsigned char*)0xFFFDC8)
+
+int
+main (void)
+{
+ MSTPCRA = MSTPCRA2 & ~0x01;
+ MSTPCRA = MSTPCRA2 ^ ~0xFE;
+ MSTPCRA = MSTPCRA2 | ~0xFE;
+ return 0;
+}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch : H8300] Bug fix for bit insn and minor tweaks to insns
2011-06-09 7:40 [Patch : H8300] Bug fix for bit insn and minor tweaks to insns Kaushik Phatak
@ 2011-06-10 18:37 ` Jeff Law
2011-06-13 10:57 ` Kaushik Phatak
2011-06-13 13:16 ` Eric Botcazou
1 sibling, 1 reply; 5+ messages in thread
From: Jeff Law @ 2011-06-10 18:37 UTC (permalink / raw)
To: Kaushik Phatak; +Cc: gcc-patches, Prafulla Thakare
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 06/09/11 01:20, Kaushik Phatak wrote:
> Hi,
> The following patch fixes an ICE that is generated when the compiler tries
> to perform bit manipulation for logical operations when the source and
> destination address does not match. The testcase is also included in the
> patch(gcc.dg).
> The additional condition in the insn takes care of the ICE which occurs at '-O1'.
> The other insn's are reordered to give preference to bit instructions using existing
> constraints.
> Ok to apply?
>
> Thanks & Regards,
> Kaushik Phatak
> www.kpitgnutools.com
>
> 2011-06-09 Kaushik Phatak <kaushik.phatak@kpitcummins.com>
>
> * config/h8300/h8300.md (bsetqi_msx, bclrqi_msx, bnotqi_msx): Added
> condition to disallow non-identical memory locations.
> (*andqi3_2, andqi3_1, iorqi3_1, xorqi3_1): Reorder insn to give
> preference to bit manipulation instructions.
> * gcc.dg/h8300-bit-insn-ice2.2: New testcase.
Can't the operand be MEM (reg) or MEM (const_int)? In which case
INTVAL (XEXP (operands[], 0)) is the wrong test since you shouldn't be
applying INTVAL to a REG. Furthermore, if you're trying to compare
CONST_INTs, those are shared and you can use pointer equality rather
than their underlying value.
Regardless, I think the right test is
rtx_equal_p (operands[0], operands[1], NULL)
With that change to bsetqi_msx, bclrqi_msx and bnotqi_msx this patch is OK.
jeff
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iQEcBAEBAgAGBQJN8mMBAAoJEBRtltQi2kC7NjUIAJDylrPPoI6YMk4lfRwclIPT
VCzk31q3DpZ3H5CEv8e6u7DmmJl9ng78uYHXejAqTbpAn/mDOtEWasBIUIhTACFF
UgH3bK3wwhF412Dbr/6ND/dPCMiImzSUR8PC8N6S31k8q5JmXIkfVfX/oKaSH/n6
9VxyIfh7PmGRqjMlH434DyvOvH+qPfN3jSNLVHDvUeVYKI6gnfXtR8tuHTlIw4LO
eiubgvmD4RXa6Nnpeuafs8GyTCRA5jbVU2jf3ZR3jBA08YpS/cSeECd19ytSdVuS
Au20hmDPStkJwr/0kw+QtqeZRg5BP4JWF0COYQJDFQVa64Gt6LEterBmntPJr8g=
=JdgV
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [Patch : H8300] Bug fix for bit insn and minor tweaks to insns
2011-06-10 18:37 ` Jeff Law
@ 2011-06-13 10:57 ` Kaushik Phatak
2011-06-13 15:24 ` H.J. Lu
0 siblings, 1 reply; 5+ messages in thread
From: Kaushik Phatak @ 2011-06-13 10:57 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches, Prafulla Thakare
Hi Jeff,
Thanks for the quick review.
>> the right test is rtx_equal_p(operands[0], operands[1])
Committed with above changes to the bsetqi_msx, bclrqi_msx and bnotqi_msx patterns.
Thanks & Regards,
Kaushik Phatak
www.kpitgnutools.com
-----Original Message-----
From: Jeff Law [mailto:law@redhat.com]
Sent: 11 June 2011 00:01
To: Kaushik Phatak
Cc: gcc-patches@gcc.gnu.org; Prafulla Thakare
Subject: Re: [Patch : H8300] Bug fix for bit insn and minor tweaks to insns
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 06/09/11 01:20, Kaushik Phatak wrote:
> Hi,
> The following patch fixes an ICE that is generated when the compiler tries
> to perform bit manipulation for logical operations when the source and
> destination address does not match. The testcase is also included in the
> patch(gcc.dg).
> The additional condition in the insn takes care of the ICE which occurs at '-O1'.
> The other insn's are reordered to give preference to bit instructions using existing
> constraints.
> Ok to apply?
>
> Thanks & Regards,
> Kaushik Phatak
> www.kpitgnutools.com
>
> 2011-06-09 Kaushik Phatak <kaushik.phatak@kpitcummins.com>
>
> * config/h8300/h8300.md (bsetqi_msx, bclrqi_msx, bnotqi_msx): Added
> condition to disallow non-identical memory locations.
> (*andqi3_2, andqi3_1, iorqi3_1, xorqi3_1): Reorder insn to give
> preference to bit manipulation instructions.
> * gcc.dg/h8300-bit-insn-ice2.2: New testcase.
Can't the operand be MEM (reg) or MEM (const_int)? In which case
INTVAL (XEXP (operands[], 0)) is the wrong test since you shouldn't be
applying INTVAL to a REG. Furthermore, if you're trying to compare
CONST_INTs, those are shared and you can use pointer equality rather
than their underlying value.
Regardless, I think the right test is
rtx_equal_p (operands[0], operands[1], NULL)
With that change to bsetqi_msx, bclrqi_msx and bnotqi_msx this patch is OK.
jeff
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iQEcBAEBAgAGBQJN8mMBAAoJEBRtltQi2kC7NjUIAJDylrPPoI6YMk4lfRwclIPT
VCzk31q3DpZ3H5CEv8e6u7DmmJl9ng78uYHXejAqTbpAn/mDOtEWasBIUIhTACFF
UgH3bK3wwhF412Dbr/6ND/dPCMiImzSUR8PC8N6S31k8q5JmXIkfVfX/oKaSH/n6
9VxyIfh7PmGRqjMlH434DyvOvH+qPfN3jSNLVHDvUeVYKI6gnfXtR8tuHTlIw4LO
eiubgvmD4RXa6Nnpeuafs8GyTCRA5jbVU2jf3ZR3jBA08YpS/cSeECd19ytSdVuS
Au20hmDPStkJwr/0kw+QtqeZRg5BP4JWF0COYQJDFQVa64Gt6LEterBmntPJr8g=
=JdgV
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch : H8300] Bug fix for bit insn and minor tweaks to insns
2011-06-09 7:40 [Patch : H8300] Bug fix for bit insn and minor tweaks to insns Kaushik Phatak
2011-06-10 18:37 ` Jeff Law
@ 2011-06-13 13:16 ` Eric Botcazou
1 sibling, 0 replies; 5+ messages in thread
From: Eric Botcazou @ 2011-06-13 13:16 UTC (permalink / raw)
To: Kaushik Phatak; +Cc: gcc-patches, law, Prafulla Thakare
> 2011-06-09 Kaushik Phatak <kaushik.phatak@kpitcummins.com>
>
> * config/h8300/h8300.md (bsetqi_msx, bclrqi_msx, bnotqi_msx): Added
> condition to disallow non-identical memory locations.
> (*andqi3_2, andqi3_1, iorqi3_1, xorqi3_1): Reorder insn to give
> preference to bit manipulation instructions.
> * gcc.dg/h8300-bit-insn-ice2.2: New testcase.
The testsuite has its own ChangeLog file, please move the last line to it.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch : H8300] Bug fix for bit insn and minor tweaks to insns
2011-06-13 10:57 ` Kaushik Phatak
@ 2011-06-13 15:24 ` H.J. Lu
0 siblings, 0 replies; 5+ messages in thread
From: H.J. Lu @ 2011-06-13 15:24 UTC (permalink / raw)
To: Kaushik Phatak; +Cc: Jeff Law, gcc-patches, Prafulla Thakare
On Mon, Jun 13, 2011 at 2:40 AM, Kaushik Phatak
<Kaushik.Phatak@kpitcummins.com> wrote:
> Hi Jeff,
> Thanks for the quick review.
>
>>> the right test is rtx_equal_p(operands[0], operands[1])
> Committed with above changes to the bsetqi_msx, bclrqi_msx and bnotqi_msx patterns.
>
Commit is incorrect. Testsuite ChangeLog entries should be in
testsuite/ChangeLog.
You have duplicated lines in gcc.dg/h8300-bit-insn-ice2.c and filename
is ChangeLog
is incorrect. I checked in this fix.
--
H.J.
---
Index: ChangeLog
===================================================================
--- ChangeLog (revision 174986)
+++ ChangeLog (working copy)
@@ -4,7 +4,6 @@
condition to disallow non-identical memory locations.
(*andqi3_2, andqi3_1, iorqi3_1, xorqi3_1): Reorder insn to give
preference to bit manipulation instructions.
- * gcc.dg/h8300-bit-insn-ice2.2: New testcase.
2011-06-13 Jan Hubicka <jh@suse.cz>
Index: testsuite/gcc.dg/h8300-bit-insn-ice2.c
===================================================================
--- testsuite/gcc.dg/h8300-bit-insn-ice2.c (revision 174986)
+++ testsuite/gcc.dg/h8300-bit-insn-ice2.c (working copy)
@@ -13,18 +13,3 @@ main (void)
MSTPCRA = MSTPCRA2 | ~0xFE;
return 0;
}
-/* { dg-skip-if "" { "h8300*-*-*" } "*" "-msx*" } */
-/* { dg-options "-O2" } */
-/* ICE for bit instruction generation using 16-bit const */
-
-#define MSTPCRA (*(volatile unsigned char*)0xFFFFC9)
-#define MSTPCRA2 (*(volatile unsigned char*)0xFFFDC8)
-
-int
-main (void)
-{
- MSTPCRA = MSTPCRA2 & ~0x01;
- MSTPCRA = MSTPCRA2 ^ ~0xFE;
- MSTPCRA = MSTPCRA2 | ~0xFE;
- return 0;
-}
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog (revision 174986)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,11 @@
+2011-06-13 H.J. Lu <hongjiu.lu@intel.com>
+
+ * gcc.dg/h8300-bit-insn-ice2.c: Remove duplicated lines.
+
+2011-06-13 Kaushik Phatak <kaushik.phatak@kpitcummins.com>
+
+ * gcc.dg/h8300-bit-insn-ice2.c: New testcase.
+
2011-06-13 Thomas Koenig <tkoenig@gcc.gnu.org>
* gfortran.dg/trim_optimize_8.f90: New test case.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-06-13 13:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-09 7:40 [Patch : H8300] Bug fix for bit insn and minor tweaks to insns Kaushik Phatak
2011-06-10 18:37 ` Jeff Law
2011-06-13 10:57 ` Kaushik Phatak
2011-06-13 15:24 ` H.J. Lu
2011-06-13 13:16 ` Eric Botcazou
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).