* [Patch : H8300] Minor bug fix for bit instructions
@ 2010-10-29 15:00 Kaushik Phatak
2010-11-02 3:58 ` Jeff Law
0 siblings, 1 reply; 5+ messages in thread
From: Kaushik Phatak @ 2010-10-29 15:00 UTC (permalink / raw)
To: gcc-patches; +Cc: Jeff Law, Prafulla Thakare
Hi,
The patch below fixes an ICE caused when 16-bit constants are used
as operand 2 in logical operation.
MSTP.CRA.WORD |= 0x40; -> This is OK
MSTP.CRA.WORD |= 0x4000; -> This generates ICE
Operand 2 is right shifted by 8 so that it is in valid range to
perform bit-operation in QI mode.
The 'abs' is used for specific cases for constants like 0xFEFF (-257).
Regression done for h8300-elf-* and no new regressions found.
Please comment.
Regards,
Kaushik Phatak
www.kpitgnutools.com
Changelog:
2010-10-29 Kaushik Phatak <kaushik.phatak@kpitcummins.com>
* config/h8300/h8300.md (define_splits) : Add condition for
16-bit const operands.
diff -upr trunk.orig/gcc/config/h8300/h8300.md trunk/gcc/config/h8300/h8300.md
--- trunk.orig/gcc/config/h8300/h8300.md 2010-08-26 20:55:19.000000000 +0530
+++ trunk/gcc/config/h8300/h8300.md 2010-10-28 19:57:14.000000000 +0530
@@ -1781,8 +1781,17 @@
(and:QI (match_dup 1)
(match_dup 2)))]
{
- operands[0] = adjust_address (operands[0], QImode, 1);
- operands[1] = adjust_address (operands[1], QImode, 1);
+ if(abs(INTVAL(operands[2])) > 0xFF)
+ {
+ operands[0] = adjust_address (operands[0], QImode, 0);
+ operands[1] = adjust_address (operands[1], QImode, 0);
+ operands[2] = GEN_INT ((INTVAL(operands[2])) >> 8);
+ }
+ else
+ {
+ operands[0] = adjust_address (operands[0], QImode, 1);
+ operands[1] = adjust_address (operands[1], QImode, 1);
+ }
})
(define_insn "bclrhi_msx"
@@ -1916,8 +1925,17 @@
(ior:QI (match_dup 1)
(match_dup 2)))]
{
- operands[0] = adjust_address (operands[0], QImode, 1);
- operands[1] = adjust_address (operands[1], QImode, 1);
+ if(abs(INTVAL(operands[2])) > 0xFF)
+ {
+ operands[0] = adjust_address (operands[0], QImode, 0);
+ operands[1] = adjust_address (operands[1], QImode, 0);
+ operands[2] = GEN_INT ((INTVAL(operands[2])) >> 8);
+ }
+ else
+ {
+ operands[0] = adjust_address (operands[0], QImode, 1);
+ operands[1] = adjust_address (operands[1], QImode, 1);
+ }
})
(define_insn "bsethi_msx"
@@ -1982,8 +2000,17 @@
(xor:QI (match_dup 1)
(match_dup 2)))]
{
- operands[0] = adjust_address (operands[0], QImode, 1);
- operands[1] = adjust_address (operands[1], QImode, 1);
+ if(abs(INTVAL(operands[2])) > 0xFF)
+ {
+ operands[0] = adjust_address (operands[0], QImode, 0);
+ operands[1] = adjust_address (operands[1], QImode, 0);
+ operands[2] = GEN_INT ((INTVAL(operands[2])) >> 8);
+ }
+ else
+ {
+ operands[0] = adjust_address (operands[0], QImode, 1);
+ operands[1] = adjust_address (operands[1], QImode, 1);
+ }
})
(define_insn "bnothi_msx"
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Patch : H8300] Minor bug fix for bit instructions
2010-10-29 15:00 [Patch : H8300] Minor bug fix for bit instructions Kaushik Phatak
@ 2010-11-02 3:58 ` Jeff Law
2010-11-08 9:06 ` Kaushik Phatak
0 siblings, 1 reply; 5+ messages in thread
From: Jeff Law @ 2010-11-02 3:58 UTC (permalink / raw)
To: Kaushik Phatak; +Cc: gcc-patches, Prafulla Thakare
On 10/29/10 08:17, Kaushik Phatak wrote:
> Hi,
> The patch below fixes an ICE caused when 16-bit constants are used
> as operand 2 in logical operation.
>
> MSTP.CRA.WORD |= 0x40; -> This is OK
> MSTP.CRA.WORD |= 0x4000; -> This generates ICE
>
> Operand 2 is right shifted by 8 so that it is in valid range to
> perform bit-operation in QI mode.
> The 'abs' is used for specific cases for constants like 0xFEFF (-257).
>
> Regression done for h8300-elf-* and no new regressions found.
> Please comment.
>
> Regards,
> Kaushik Phatak
> www.kpitgnutools.com
>
> Changelog:
>
> 2010-10-29 Kaushik Phatak<kaushik.phatak@kpitcummins.com>
>
> * config/h8300/h8300.md (define_splits) : Add condition for
> 16-bit const operands.
A nit, be more specific about which splitters. List each one
separately. You might call them "and with single_zero" "ior with
single_one" and "xor with single_one" splitters or something like that.
>
> diff -upr trunk.orig/gcc/config/h8300/h8300.md trunk/gcc/config/h8300/h8300.md
> --- trunk.orig/gcc/config/h8300/h8300.md 2010-08-26 20:55:19.000000000 +0530
> +++ trunk/gcc/config/h8300/h8300.md 2010-10-28 19:57:14.000000000 +0530
> @@ -1781,8 +1781,17 @@
> (and:QI (match_dup 1)
> (match_dup 2)))]
> {
> - operands[0] = adjust_address (operands[0], QImode, 1);
> - operands[1] = adjust_address (operands[1], QImode, 1);
> + if(abs(INTVAL(operands[2]))> 0xFF)
Formatting nits. You want a space after the "if" and before each open
parenthesis.
I'd really like to see a test included for the testsuite. OK with those
changes.
Jeff
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [Patch : H8300] Minor bug fix for bit instructions
2010-11-02 3:58 ` Jeff Law
@ 2010-11-08 9:06 ` Kaushik Phatak
2010-11-09 15:28 ` Jeff Law
0 siblings, 1 reply; 5+ messages in thread
From: Kaushik Phatak @ 2010-11-08 9:06 UTC (permalink / raw)
To: gcc-patches; +Cc: Jeff Law, Prafulla Thakare
Hi Jeff,
Thanks for your detailed review.
Please find below an updated patch with the suggested changes,
- Changelog more explicit
- formatting corrections
- Testcase added to gcc.dg
Please let me know if these are OK.
Thanks,
Kaushik Phatak
www.kpitgnutools.com
Changelog:
2010-11-08 Kaushik Phatak <kaushik.phatak@kpitcummins.com>
* config/h8300/h8300.md (define_split) : Add condition for
"and with single_zero" splitter to handle 16-bit const operands.
* config/h8300/h8300.md (define_split) : Add condition for
"ior with single_one" splitter to handle 16-bit const operands.
* config/h8300/h8300.md (define_split) : Add condition for
"xor with single_one" splitter to handle 16-bit const operands.
diff -upr trunk.orig/gcc/config/h8300/h8300.md trunk/gcc/config/h8300/h8300.md
--- trunk.orig/gcc/config/h8300/h8300.md 2010-08-26 20:55:19.000000000 +0530
+++ trunk/gcc/config/h8300/h8300.md 2010-10-28 19:57:14.000000000 +0530
@@ -1781,8 +1781,17 @@
(and:QI (match_dup 1)
(match_dup 2)))]
{
- operands[0] = adjust_address (operands[0], QImode, 1);
- operands[1] = adjust_address (operands[1], QImode, 1);
+ if (abs (INTVAL (operands[2])) > 0xFF)
+ {
+ operands[0] = adjust_address (operands[0], QImode, 0);
+ operands[1] = adjust_address (operands[1], QImode, 0);
+ operands[2] = GEN_INT ((INTVAL (operands[2])) >> 8);
+ }
+ else
+ {
+ operands[0] = adjust_address (operands[0], QImode, 1);
+ operands[1] = adjust_address (operands[1], QImode, 1);
+ }
})
(define_insn "bclrhi_msx"
@@ -1916,8 +1925,17 @@
(ior:QI (match_dup 1)
(match_dup 2)))]
{
- operands[0] = adjust_address (operands[0], QImode, 1);
- operands[1] = adjust_address (operands[1], QImode, 1);
+ if (abs (INTVAL (operands[2])) > 0xFF)
+ {
+ operands[0] = adjust_address (operands[0], QImode, 0);
+ operands[1] = adjust_address (operands[1], QImode, 0);
+ operands[2] = GEN_INT ((INTVAL (operands[2])) >> 8);
+ }
+ else
+ {
+ operands[0] = adjust_address (operands[0], QImode, 1);
+ operands[1] = adjust_address (operands[1], QImode, 1);
+ }
})
(define_insn "bsethi_msx"
@@ -1982,8 +2000,17 @@
(xor:QI (match_dup 1)
(match_dup 2)))]
{
- operands[0] = adjust_address (operands[0], QImode, 1);
- operands[1] = adjust_address (operands[1], QImode, 1);
+ if (abs (INTVAL (operands[2])) > 0xFF)
+ {
+ operands[0] = adjust_address (operands[0], QImode, 0);
+ operands[1] = adjust_address (operands[1], QImode, 0);
+ operands[2] = GEN_INT ((INTVAL (operands[2])) >> 8);
+ }
+ else
+ {
+ operands[0] = adjust_address (operands[0], QImode, 1);
+ operands[1] = adjust_address (operands[1], QImode, 1);
+ }
})
(define_insn "bnothi_msx"
diff -upN trunk.orig/gcc/testsuite/gcc.dg/h8300-bit-insn-ice.c trunk/gcc/testsuite/gcc.dg/h8300-bit-insn-ice.c
--- trunk.orig/gcc/testsuite/gcc.dg/h8300-bit-insn-ice.c 1970-01-01 05:30:00.000000000 +0530
+++ trunk/gcc/testsuite/gcc.dg/h8300-bit-insn-ice.c 2010-11-08 11:43:23.000000000 +0530
@@ -0,0 +1,39 @@
+/* { dg-skip-if "" { "h8300*-*-*" } "*" "-msx*" } */
+/* ICE for bit instruction generation using 16-bit const */
+
+__extension__ struct st_mstp
+{
+ union
+ {
+ unsigned short WORD;
+ struct
+ {
+ unsigned char ACSE:1;
+ unsigned char _EXDMAC:1;
+ unsigned char _DMAC:1;
+ unsigned char _DTC:1;
+ unsigned char:2;
+ unsigned char _TMR23:1;
+ unsigned char _TMR01:1;
+ unsigned char:2;
+ unsigned char _DA:1;
+ unsigned char:1;
+ unsigned char _AD:1;
+ unsigned char:1;
+ unsigned char _TPUU:1;
+ unsigned char _TPUL:1;
+ } BIT;
+ } CRA;
+};
+#define MSTP (*(volatile struct st_mstp *)0xFFFDC8)
+#define MSTPA_EXDMA 0x4000
+#define MSTPA_AND 0xFEFF
+
+int
+main ()
+{
+ MSTP.CRA.WORD |= MSTPA_EXDMA;
+ MSTP.CRA.WORD ^= MSTPA_EXDMA;
+ MSTP.CRA.WORD &= MSTPA_AND;
+ return 0;
+}
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Patch : H8300] Minor bug fix for bit instructions
2010-11-08 9:06 ` Kaushik Phatak
@ 2010-11-09 15:28 ` Jeff Law
2010-12-14 7:25 ` Kaushik Phatak
0 siblings, 1 reply; 5+ messages in thread
From: Jeff Law @ 2010-11-09 15:28 UTC (permalink / raw)
To: Kaushik Phatak; +Cc: gcc-patches, Prafulla Thakare
On 11/08/10 00:44, Kaushik Phatak wrote:
> Hi Jeff,
> Thanks for your detailed review.
> Please find below an updated patch with the suggested changes,
> - Changelog more explicit
> - formatting corrections
> - Testcase added to gcc.dg
>
> Please let me know if these are OK.
>
> Thanks,
> Kaushik Phatak
> www.kpitgnutools.com
>
>
> Changelog:
>
> 2010-11-08 Kaushik Phatak<kaushik.phatak@kpitcummins.com>
>
> * config/h8300/h8300.md (define_split) : Add condition for
> "and with single_zero" splitter to handle 16-bit const operands.
> * config/h8300/h8300.md (define_split) : Add condition for
> "ior with single_one" splitter to handle 16-bit const operands.
> * config/h8300/h8300.md (define_split) : Add condition for
> "xor with single_one" splitter to handle 16-bit const operands.
OK. Please install. If you don't have write privs for the repository,
go ahead and sign up via:
http://sourceware.org/cgi-bin/pdw/ps_form.cgi
Thanks,
Jeff
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-12-14 7:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-29 15:00 [Patch : H8300] Minor bug fix for bit instructions Kaushik Phatak
2010-11-02 3:58 ` Jeff Law
2010-11-08 9:06 ` Kaushik Phatak
2010-11-09 15:28 ` Jeff Law
2010-12-14 7:25 ` Kaushik Phatak
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).