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
Ian Lance Taylor <email@example.com> writes:
> Richard Sandiford <firstname.lastname@example.org> writes:
>> The second sentence is why I don't think this patch is a good idea. I
>> don't want to see us add new target-independent expansions to the MIPS
>> backend. If you want to change the way that ffs is implemented, please
>> either provide this expansion in the tree->rtl expanders or do it as
>> a tree-level reduction in the way you suggest.
> What I originally wanted to do was to simply eliminate the ffs<mode>2
> insn from mips.md. Eric thought that would be a bad idea, because for
> some programs it would introduce a library call where there wasn't one
> I don't actually think it's a good idea to expand ffs inline into a
> loop. I don't think that is a choice the compiler should be making,
> so I don't think it is really appropriate to do it in generic code.
> As far as I know we do not currently expand any other builtin
> functions into loops.
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.