[PATCH] Fix CSE bogus RTL creation
Mark Mitchell
mark@codesourcery.com
Tue Jun 15 14:52:00 GMT 2010
Steven Bosscher wrote:
>>> 2010-06-14 Paul Brook <paul@codesourcery.com>
>>>
>>> gcc/
>>> * cse.c (cse_process_notes_1): Call simplify_rtx is a substitution
>>> was made.
> How is this obvious?
> It makes me suspect that this change only papers over a bug elsewhere,
> where a note replacement in an insn is not validated properly.
I think we worry too much about that. Or, rather, we confuse
*suboptimal* patches with *wrong* patches.
A wrong patch is one that will make the compiler generate wrong code, or
slow code, or cause an ICE, or do some other user-observable bad thing.
A suboptimal patch is one for which there exists a superior
implementation technique, possibly involving some much larger reworking
of the sourcebase, that would produce a solution to more problems, a
more elegant solution to the same problem, or in some other way be superior.
Of course, I am all for superior patches, good infrastructure,
clean-ups, and so forth. But, it seemed clear to me that (a)
validate_change wasn't in the business of making canonical RTL, and (b)
simplify_rtx was, and (c) this patch made the compiler better from a
user-observable point of view, in that it eliminated crashes, and (d)
there had been some previous discussion suggesting this approach, and
(e) I couldn't see any user-observable harm it would do, and (f) it was
a small local change.
I think that should be good enough. Now, of course, if you have an idea
about how to do it better, I think that's great! A more general, more
elegant solution would be terrific. But, I don't want perfect to be the
enemy of good.
--
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713
More information about the Gcc-patches
mailing list