This is the mail archive of the
mailing list for the GCC project.
Re: PATCH RFA: Change MIPS ffs<mode>2 from define_insn to define_expand
Richard Sandiford <email@example.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
> 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
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
2005-07-10 Ian Lance Taylor <firstname.lastname@example.org>
* config/mips/mips.md (ffs<mode>2): Remove.
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
- [(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"))]
- if (optimize && find_reg_note (insn, REG_DEAD, operands))
- return "%(\
- return "%(\
- [(set_attr "type" "multi")
- (set_attr "mode" "<MODE>")
- (set_attr "length" "28")])
;; Count leading zeroes.