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] MIPS: mips16e machine patterns - zeb/zeh seb/seh


> > I've added the command line option of -mips16e to turn on the generation
> > of these instructions.
> > A regression test case is also includes.
> 
> I suppose there's the question of whether this should be handled by a
> separate switch (-mips16e, like you've done) or whether the availability
> of MIPS16e should be inferred from the combination of -mips16 and the
> -march setting.

> I've no strong opinion either way, but I'd like to make sure that the
> possibility has been considered.

yes, this is a good question.  Well, mips16e is the 16bit ASE required
for mips32/64.  So if compiling for mips32/64 and -mips16, it would
imply -mips16e.  I guess the -mips16e option is useful for non mips32/64
targets that decides to implement the full mips16e set, though we could
do without it.  Its probably good to have it because it'll be a similar
option in gas.  Anyone else have any opionions on this?

> > Index: gcc/testsuite/gcc.target/mips/mips.exp
> > ===================================================================
> > RCS file: /cvs/gcc/gcc/gcc/testsuite/gcc.target/mips/mips.exp,v
> > retrieving revision 1.2
> > diff -c -p -b -r1.2 mips.exp
> > *** gcc/testsuite/gcc.target/mips/mips.exp	18 Apr 2005 20:34:36 -0000	1.2
> > --- gcc/testsuite/gcc.target/mips/mips.exp	7 Jun 2005 13:42:46 -0000
> > *************** proc dg-mips-options {args} {
> > *** 152,159 ****
> >   	    if {$mips_mips16 || ($arch != $mips_arch && $mips_forced_isa)} {
> >   		set matches 0
> >   	    }
> > ! 	} elseif {[regexp -- {^-mips(.*)} $flag dummy isa] && $isa != 16} {
> > ! 	    if {$mips_mips16 || ($isa != $mips_isa && $mips_forced_isa)} {
> >   		set matches 0
> >   	    }
> >   	} elseif {[regexp -- {^-m(hard|soft)-float} $flag dummy float]} {
> > --- 152,164 ----
> >   	    if {$mips_mips16 || ($arch != $mips_arch && $mips_forced_isa)} {
> >   		set matches 0
> >   	    }
> > ! 	} elseif {[regexp -- {^-mips(.*)} $flag dummy isa]} {
> > ! 	    if {$isa == "16e"} {
> > ! 	        if {$mips_isa < 32} {
> > ! 		    set matches 0 
> > !                 }            
> > ! 	    } elseif {$isa != 16 && ($mips_mips16 
> > !                       || ($isa != $mips_isa && $mips_forced_isa))} {
> >   		set matches 0
> >   	    }
> >   	} elseif {[regexp -- {^-m(hard|soft)-float} $flag dummy float]} {
> 
> Why is this necessary?  You've coded -mips16e so that it works for any
> -march setting, so I would have expected it to be case of extending
> "isa != 16" to something like "![string match 16* $isa]".
>
> (Note that the script doesn't cope with -mips16 for dg-do run tests,
> but we don't have any of those, and your new one is just a compile
> test too.)
> 

probably not critical for this test case.  but generally, if the user
gives -mips16 as the flag to runtest, you don't want to do the -mips16e
only tests unless -mips16 == -mips16e.  You only wanted to run this test
if the underlying ISA supports it, as its still a mips16e only test. It
just happens to work for other ISAs too since its just a compile test.
of course this is more relevent if you need to execute the test, which
you've already mentioned.  I am happy to recode it if you think it would
be more appropriate to leave out the extra tests for now.
In case you are wondering, I shall be submiting a patch to the GNU
simulator to support the new mips16e instructions in the coming weeks.

David.


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