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 RFA: Change MIPS ffs<mode>2 from define_insn to define_expand


Richard Sandiford <richard@codesourcery.com> writes:

> Yeah, for what it's worth, I largely agree.  If it had been up to me,
> I'd have been very open to your first idea of dropping the patterns
> altogether.
> 
> If we do expand inline, I think it ought to be done according to some
> sort of cost model.  The current patterns are trivially broken for -Os
> on mips*-elf because the call sequence would be shorter than the inline
> loop.  The trade-offs for -O2 are more complex, and that's a good reason
> why they shouldn't be duplicated (or ignored) by backends.  For example,
> in the presence of profile-directed inlining, we might want to handle
> "cold" and "hot" calls differently.
> 
> I'd consider it a bug that the ffs "optimisation" produces strictly
> bigger code for -Os.  And given that there's nothing particularly
> MIPS-specific in what the patterns are doing, I think that removing
> them is a good fix.  Anyone who believes that an inline loop should
> be used, and who's sufficiently motivated to do it where it belongs,
> can do that as a follow-on patch.

Well, I tested the appended patch on the MIPS simulator.  It works
fine.

If we remove the ffs insn pattern for any case, then I would argue
strongly that we should remove it for all cases.  The only reason to
keep ffs is to avoid introducing a new library call in existing code.
If we give up on maintaining that principle, then I think that it
makes no sense to keep generic code like this in the MIPS backend.

Or, if we keep the ffs insn pattern for all cases, then I think my
earlier patch, to use a define_expand, is a strict improvement over
the current code.  While it would be possible to write it in a target
independent manner, I don't think it would be appropriate in general
to expand ffs into an inline loop; I think we are only considering in
this case to maintain a historical MIPS-specific feature.  I think the
only reasonable improvement over using a define_expand in mips.md
would be to introduce a new framework for target specific folding of
builtin functions.  But I'm not sure what such a framework would be
useful for in general.

So I think either this patch or the previous one is an improvement,
and I think you and Eric should pick one, or should describe a
reasonable alternative.

Thanks.

Ian


2005-07-10  Ian Lance Taylor  <ian@airs.com>

	* config/mips/mips.md (ffs<mode>2): Remove.


Index: config/mips/mips.md
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/mips/mips.md,v
retrieving revision 1.325
diff -p -u -r1.325 mips.md
--- config/mips/mips.md	8 Jul 2005 05:24:51 -0000	1.325
+++ config/mips/mips.md	11 Jul 2005 04:50:24 -0000
@@ -1906,45 +1906,6 @@
    (set_attr "mode" "<UNITMODE>")])
 
 ;;
-;;  ....................
-;;
-;;	FIND FIRST BIT INSTRUCTION
-;;
-;;  ....................
-;;
-
-(define_insn "ffs<mode>2"
-  [(set (match_operand:GPR 0 "register_operand" "=&d")
-	(ffs:GPR (match_operand:GPR 1 "register_operand" "d")))
-   (clobber (match_scratch:GPR 2 "=&d"))
-   (clobber (match_scratch:GPR 3 "=&d"))]
-  "!TARGET_MIPS16"
-{
-  if (optimize && find_reg_note (insn, REG_DEAD, operands[1]))
-    return "%(\
-move\t%0,%.\;\
-beq\t%1,%.,2f\n\
-%~1:\tand\t%2,%1,0x0001\;\
-<d>addu\t%0,%0,1\;\
-beq\t%2,%.,1b\;\
-<d>srl\t%1,%1,1\n\
-%~2:%)";
-
-  return "%(\
-move\t%0,%.\;\
-move\t%3,%1\;\
-beq\t%3,%.,2f\n\
-%~1:\tand\t%2,%3,0x0001\;\
-<d>addu\t%0,%0,1\;\
-beq\t%2,%.,1b\;\
-<d>srl\t%3,%3,1\n\
-%~2:%)";
-}
-  [(set_attr "type" "multi")
-   (set_attr "mode" "<MODE>")
-   (set_attr "length" "28")])
-
-;;
 ;;  ...................
 ;;
 ;;  Count leading zeroes.


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