* RX: Fix LT and GE comparisons
@ 2011-02-03 11:40 Nick Clifton
2011-02-04 18:19 ` Richard Henderson
0 siblings, 1 reply; 8+ messages in thread
From: Nick Clifton @ 2011-02-03 11:40 UTC (permalink / raw)
To: gcc-patches
Hi Guys,
I am checking in the patch below to fix the LT and GE comparisons on
the RX target. These need the Overflow bit to be valid in the
processor status word.
The patch fixes 27 failures in the gcc testsuite.
Cheers
Nick
gcc/ChangeLog
2011-02-03 Nick Clifton <nickc@redhat.com>
* config/rx/predicates.md (rx_zs_comparison_operator): Remove
lt and ge.
* config/rx/rx.md (abssi2_flags): Use CC_ZSmode rather than
CC_ZSOmode.
* config/rx/rx.c (rx_print_operand): Use "lt" and "ge" suffixes
instead of "n" and "pz".
(flags_from_code): LT and GE tests need CC_FLAG_O as well as
CC_FLAG_S.
Index: gcc/config/rx/predicates.md
===================================================================
--- gcc/config/rx/predicates.md (revision 169784)
+++ gcc/config/rx/predicates.md (working copy)
@@ -284,7 +284,7 @@
)
(define_predicate "rx_zs_comparison_operator"
- (match_code "eq,ne,lt,ge")
+ (match_code "eq,ne")
)
;; GT and LE omitted due to operand swap required.
Index: gcc/config/rx/rx.md
===================================================================
--- gcc/config/rx/rx.md (revision 169784)
+++ gcc/config/rx/rx.md (working copy)
@@ -797,7 +797,10 @@
(set (reg CC_REG)
(compare (abs:SI (match_dup 1))
(const_int 0)))]
- "reload_completed && rx_match_ccmode (insn, CC_ZSOmode)"
+ ;; Note - although the ABS instruction does set the O bit in the processor
+ ;; status word, it does not do so in a way that is comparable with the CMP
+ ;; instruction. Hence we use CC_ZSmode rather than CC_ZSOmode.
+ "reload_completed && rx_match_ccmode (insn, CC_ZSmode)"
"@
abs\t%0
abs\t%1, %0"
Index: gcc/config/rx/rx.c
===================================================================
--- gcc/config/rx/rx.c (revision 169784)
+++ gcc/config/rx/rx.c (working copy)
@@ -450,10 +450,10 @@
switch (code)
{
case LT:
- ret = "n";
+ ret = "lt";
break;
case GE:
- ret = "pz";
+ ret = "ge";
break;
case GT:
ret = "gt";
@@ -2625,7 +2625,7 @@
{
case LT:
case GE:
- return CC_FLAG_S;
+ return CC_FLAG_S | CC_FLAG_O;
case GT:
case LE:
return CC_FLAG_S | CC_FLAG_O | CC_FLAG_Z;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: RX: Fix LT and GE comparisons
2011-02-03 11:40 RX: Fix LT and GE comparisons Nick Clifton
@ 2011-02-04 18:19 ` Richard Henderson
2011-02-06 17:45 ` Richard Henderson
2011-02-07 15:04 ` Nick Clifton
0 siblings, 2 replies; 8+ messages in thread
From: Richard Henderson @ 2011-02-04 18:19 UTC (permalink / raw)
To: Nick Clifton; +Cc: gcc-patches
On 02/03/2011 03:42 AM, Nick Clifton wrote:
> (define_predicate "rx_zs_comparison_operator"
> - (match_code "eq,ne,lt,ge")
> + (match_code "eq,ne")
Err... why? One can perform X < 0 and X >= 0 with just the sign bit.
> case LT:
> - ret = "n";
> + ret = "lt";
> break;
> case GE:
> - ret = "pz";
> + ret = "ge";
One can use a slightly more complicated expression here to accommodate
the above. C.f. the mn10300 port:
case GE:
/* bge is smaller than bnc. */
str = (have_flags & CC_FLAG_V ? "ge" : "nc");
A construct like this would allow you to use "n" when the O flag is
not present, and "lt" when a full cmp opcode is in use.
> case LT:
> case GE:
> - return CC_FLAG_S;
> + return CC_FLAG_S | CC_FLAG_O;
I suppose one could verify that Y is const0_rtx in rx_select_cc_mode before
selecting CC_FLAG_S for these codes. I thought it was already always zero
though, thus the trivial implementation of that routine.
Can you point me to one of the tests that are fixed by this change?
r~
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: RX: Fix LT and GE comparisons
2011-02-04 18:19 ` Richard Henderson
@ 2011-02-06 17:45 ` Richard Henderson
2011-02-07 17:04 ` Nick Clifton
2011-02-08 12:31 ` Nick Clifton
2011-02-07 15:04 ` Nick Clifton
1 sibling, 2 replies; 8+ messages in thread
From: Richard Henderson @ 2011-02-06 17:45 UTC (permalink / raw)
To: Nick Clifton; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 525 bytes --]
On 02/04/2011 10:19 AM, Richard Henderson wrote:
> I suppose one could verify that Y is const0_rtx in rx_select_cc_mode before
> selecting CC_FLAG_S for these codes. I thought it was already always zero
> though, thus the trivial implementation of that routine.
>
> Can you point me to one of the tests that are fixed by this change?
Irritatingly, I didn't copy the dejagnu magic for rx-sim onto this machine
before I ran tests last night, but see if this patch doesn't solve the
problem you were trying to address.
r~
[-- Attachment #2: z --]
[-- Type: text/plain, Size: 1914 bytes --]
diff --git a/gcc/config/rx/predicates.md b/gcc/config/rx/predicates.md
index 82cac42..77b3353 100644
--- a/gcc/config/rx/predicates.md
+++ b/gcc/config/rx/predicates.md
@@ -284,7 +284,7 @@
)
(define_predicate "rx_zs_comparison_operator"
- (match_code "eq,ne")
+ (match_code "eq,ne,lt,ge")
)
;; GT and LE omitted due to operand swap required.
diff --git a/gcc/config/rx/rx.c b/gcc/config/rx/rx.c
index 839523f..e01f453 100644
--- a/gcc/config/rx/rx.c
+++ b/gcc/config/rx/rx.c
@@ -447,13 +447,14 @@ rx_print_operand (FILE * file, rtx op, int letter)
}
else
{
+ unsigned int flags = flags_from_mode (mode);
switch (code)
{
case LT:
- ret = "lt";
+ ret = (flags & CC_FLAG_O ? "lt" : "n");
break;
case GE:
- ret = "ge";
+ ret = (flags & CC_FLAG_O ? "ge" : "pz");
break;
case GT:
ret = "gt";
@@ -482,8 +483,7 @@ rx_print_operand (FILE * file, rtx op, int letter)
default:
gcc_unreachable ();
}
- gcc_checking_assert ((flags_from_code (code)
- & ~flags_from_mode (mode)) == 0);
+ gcc_checking_assert ((flags_from_code (code) & ~flags) == 0);
}
fputs (ret, file);
break;
@@ -2625,7 +2625,7 @@ flags_from_code (enum rtx_code code)
{
case LT:
case GE:
- return CC_FLAG_S | CC_FLAG_O;
+ return CC_FLAG_S;
case GT:
case LE:
return CC_FLAG_S | CC_FLAG_O | CC_FLAG_Z;
@@ -2666,11 +2666,14 @@ rx_cc_modes_compatible (enum machine_mode m1, enum machine_mode m2)
/* Return the minimal CC mode needed to implement (CMP_CODE X Y). */
enum machine_mode
-rx_select_cc_mode (enum rtx_code cmp_code, rtx x, rtx y ATTRIBUTE_UNUSED)
+rx_select_cc_mode (enum rtx_code cmp_code, rtx x, rtx y)
{
if (GET_MODE_CLASS (GET_MODE (x)) == MODE_FLOAT)
return CC_Fmode;
+ if (y != const0_rtx)
+ return CCmode;
+
return mode_from_flags (flags_from_code (cmp_code));
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: RX: Fix LT and GE comparisons
2011-02-04 18:19 ` Richard Henderson
2011-02-06 17:45 ` Richard Henderson
@ 2011-02-07 15:04 ` Nick Clifton
1 sibling, 0 replies; 8+ messages in thread
From: Nick Clifton @ 2011-02-07 15:04 UTC (permalink / raw)
To: Richard Henderson; +Cc: gcc-patches
Hi Richard,
> Can you point me to one of the tests that are fixed by this change?
PASS: gcc.c-torture/execute/20050104-1.c execution, -O0
And at higher optimization levels. There is also:
gcc.c-torture/execute/cmpsi-2.c
gcc.c-torture/execute/int-compare.c
gcc.c-torture/execute/pr28651.c
I am about to apply your patch and rerun the testsuite to see what
effect it has.
Cheers
Nick
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: RX: Fix LT and GE comparisons
2011-02-06 17:45 ` Richard Henderson
@ 2011-02-07 17:04 ` Nick Clifton
2011-02-08 12:31 ` Nick Clifton
1 sibling, 0 replies; 8+ messages in thread
From: Nick Clifton @ 2011-02-07 17:04 UTC (permalink / raw)
To: Richard Henderson; +Cc: gcc-patches
Hi Richard,
> Irritatingly, I didn't copy the dejagnu magic for rx-sim onto this machine
> before I ran tests last night, but see if this patch doesn't solve the
> problem you were trying to address.
Hmm, it does fix the same testcases that my patch fixed. But it also
appears to introduce a new failure:
gcc/testsuite/gcc.dg/tree-ssa/pr42327.c: In function 'foo':
gcc/testsuite/gcc.dg/tree-ssa/pr42327.c:4:6: internal compiler error:
Segmentation fault
However this appears to be a memory corruption in the omega code, which
is unlikely to be connected with RX comparison instructions. So I think
that maybe it is just a case that a new bug has been introduced into the
gcc sources between the time that I ran my pre-patch tests and today.
I'll try reverting your patch and my patch and then running the
testsuite again to make absolutely sure.
Cheers
Nick
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: RX: Fix LT and GE comparisons
2011-02-06 17:45 ` Richard Henderson
2011-02-07 17:04 ` Nick Clifton
@ 2011-02-08 12:31 ` Nick Clifton
2011-02-09 17:37 ` Richard Henderson
1 sibling, 1 reply; 8+ messages in thread
From: Nick Clifton @ 2011-02-08 12:31 UTC (permalink / raw)
To: Richard Henderson; +Cc: gcc-patches
Hi Richard,
> Irritatingly, I didn't copy the dejagnu magic for rx-sim onto this machine
> before I ran tests last night, but see if this patch doesn't solve the
> problem you were trying to address.
I have now rerun the gcc testsuite without your patch applied or mine,
and unfortunately gcc.dg/tree-ssa/pr42327.c passes. So there must be
something in your patch that is triggering this failure. Any ideas ?
Cheers
Nick
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: RX: Fix LT and GE comparisons
2011-02-08 12:31 ` Nick Clifton
@ 2011-02-09 17:37 ` Richard Henderson
2011-02-10 14:50 ` Nick Clifton
0 siblings, 1 reply; 8+ messages in thread
From: Richard Henderson @ 2011-02-09 17:37 UTC (permalink / raw)
To: Nick Clifton; +Cc: gcc-patches
On 02/08/2011 04:32 AM, Nick Clifton wrote:
> I have now rerun the gcc testsuite without your patch applied or
> mine, and unfortunately gcc.dg/tree-ssa/pr42327.c passes. So there
> must be something in your patch that is triggering this failure. Any
> ideas ?
None. I can't reproduce this with r169926 with and without my patch.
r~
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: RX: Fix LT and GE comparisons
2011-02-09 17:37 ` Richard Henderson
@ 2011-02-10 14:50 ` Nick Clifton
0 siblings, 0 replies; 8+ messages in thread
From: Nick Clifton @ 2011-02-10 14:50 UTC (permalink / raw)
To: Richard Henderson; +Cc: gcc-patches
Hi Richard,
> None. I can't reproduce this with r169926 with and without my patch.
Hmm, must be something in my local setup then. Could you check your
patch in please ?
Cheers
Nick
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-02-10 14:50 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-03 11:40 RX: Fix LT and GE comparisons Nick Clifton
2011-02-04 18:19 ` Richard Henderson
2011-02-06 17:45 ` Richard Henderson
2011-02-07 17:04 ` Nick Clifton
2011-02-08 12:31 ` Nick Clifton
2011-02-09 17:37 ` Richard Henderson
2011-02-10 14:50 ` Nick Clifton
2011-02-07 15:04 ` Nick Clifton
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).