This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Add -meb to two Octeon tests


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" } } */
 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]