This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Add -meb to two Octeon tests
- From: Adam Nemet <anemet at caviumnetworks dot com>
- To: Richard Sandiford <rdsandiford at googlemail dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Mon, 6 Oct 2008 15:56:40 -0700
- Subject: Re: [PATCH] Add -meb to two Octeon tests
- References: <18654.3397.232137.534555@foo.home> <87abdouftw.fsf@firetop.home>
Richard Sandiford writes:
> Adam Nemet <anemet@caviumnetworks.com> writes:
> > There are two tests that fail when compiled for little-endian.
> >
> > octeon-exts-2.c is more obvious. In this structure:
> >
> > struct bar
> > {
> > unsigned long long a:1;
> > long long b:14;
> > unsigned long long c:48;
> > long long d:1;
> > };
> >
> > when d is extracted, it is the bottom bit in big-endian but the top bit in
> > little-endian. If d is the top bit then signed extraction can be achieved
> > equally well with a arithmetic right shift.
>
> Since we're still supposed to be able to use this instruction for
> _some_ code on little endian targets, I'd prefer that we either:
>
> (a) apply your patch but create an -mel-friendly version too.
> (b) reverse the order of the fields when _MIPSEL is defined.
>
> The first is better really. (I know it must seem daft having
> -mel Octeon tests, but it isn't a rejected combination.)
Since one of the cases in this test is meant to verify extraction of the
bottom bit I went with (b) here. (Also note that there are other exts tests
already that are -mel-friendly.)
> > In octeon-bbit-3.c -- which is really endian-agnostic --- for both bit
> > comparisons a different insn is generated by combine than what we have bbit
> > patterns for:
> >
> > (if_then_else (ne (zero_extract:DI (subreg:DI (truncate:SI (reg:DI 196)) 0)
> > (const_int 1)
> > (const_int 0))
> > (const_int 0))
> > (label_ref 20)
> > (pc)))
> >
> > There is a pattern for this in our compiler but this is something the
> > middle-end should be able to simplify. I added this to my todo list.
> >
> > I would prefer changing this test to big-endian-only as well. My intent was
> > to guard us from regressing in this area and not to add tests that are known
> > to fail.
>
> That's fine, but please put something like the above in a comment.
Sure. Here is the updated patch.
Tested these testcases with {,-mabi=32,-mabi=n32,-mabi=64,-meb,-mel,-mel\
-mabi=32} on mips64octeon-linux-gnu.
OK to install?
Adam
* gcc.target/mips/octeon-exts-2.c: Make it work with -mel.
* gcc.target/mips/octeon-bbit-3.c: Compile with -meb. Add
comment why this is necessary.
Index: gcc.target/mips/octeon-exts-2.c
===================================================================
--- gcc.target/mips/octeon-exts-2.c (revision 140670)
+++ gcc.target/mips/octeon-exts-2.c (working copy)
@@ -4,10 +4,17 @@
struct bar
{
+#ifdef _MIPSEB
unsigned long long a:1;
long long b:14;
unsigned long long c:48;
long long d:1;
+#else
+ long long d:1;
+ unsigned long long c:48;
+ long long b:14;
+ unsigned long long a:1;
+#endif
};
NOMIPS16 int
Index: gcc.target/mips/octeon-bbit-3.c
===================================================================
--- gcc.target/mips/octeon-bbit-3.c (revision 140670)
+++ gcc.target/mips/octeon-bbit-3.c (working copy)
@@ -1,5 +1,18 @@
/* { dg-do compile } */
-/* { dg-mips-options "-O2 -march=octeon" } */
+
+/* Force big-endian because for little-endian, combine generates this:
+
+ (if_then_else (ne (zero_extract:DI (subreg:DI (truncate:SI (reg:DI 196)) 0)
+ (const_int 1)
+ (const_int 0))
+ (const_int 0))
+ (label_ref 20)
+ (pc)))
+
+ which does not get recognized as a valid bbit pattern. The
+ middle-end should be able to simplify this futher. */
+/* { dg-mips-options "-O2 -march=octeon -meb" } */
+
/* { dg-final { scan-assembler-times "\tbbit\[01\]\t|\tbgez\t" 2 } } */
/* { dg-final { scan-assembler-not "ext\t" } } */