------- Additional Comments From roger at eyesopen dot com 2005-03-09 16:13 ------- Subject: Re: [PR middle-end/18628] do not fold to label load from tablejump to reg
On 9 Mar 2005, Alexandre Oliva wrote: > This patch is meant to implement suggestion #3 proposed to fix the bug > by Roger Sayle and selected by RTH in bugzilla. So far, I've only > verified that it fixes the testcase included in the patch. > > Alexandre Oliva <[EMAIL PROTECTED]> > > PR middle-end/18628 > * cse.c (fold_rtx_mem): Instead of returning the label extracted > from a tablejump, add it as an REG_EQUAL note, if the insn loaded > from the table to a register. > (cse_insn): Don't use it as src_eqv. Thanks! OK for mainline if bootstrap and regression testing passes. Once this patch has been on mainline for a few days, to check that targets with different forms of tablejump and conditional branches don't have issues, OK to backport to the 4.0 branch. Thanks also to RTH for selecting which of the proposals in the bugzilla PR he preferred. I'll admit that I hadn't noticed he'd commented on them until you'd posted this patch. However, full credit goes to your patch. I hadn't appreciated that the problematic transformation takes place in fold_rtx_mem which has the instruction context, allowing us to perform this transformation when its safe (i.e. we directly converting a tablejump into an unconditional jump) but to avoid the problematic case of hositing a label_ref into a register that can then escape. Cool. > The thought of adding the REG_EQUAL note was to help other passes that > might want to turn the indirect jump into a direct jump. I'm not sure > this may actually happen. I'm also not sure how much this will help. If you do encounter any problems with your patch, my first instinct would be to investigate not bothering with the REG_EQUAL note. We've had issues in the past with whether label_refs in REG_EQUAL notes are counted in the label's NUSES, and similar ugly corner cases. If there's no measurable performance impact, we're probably better off without the risk [on the 4.0 branch at least]. > Bootstrap and regtesting starting shortly. Ok to install if they > pass? Please forgive me for commenting on this, but it's kind of a pet peeve. There really are no patches so urgent that they can't be bootstrapped and regression tested before posting to gcc-patches. Even "obvious" fixes to target-independent bootstrap failures can affort the few hours it takes to confirm changes work and are safe. Indeed the language (and emphasis) in contribute.html is (are) quite clear on the matter. My apologies for bringing this up now as you're certainly not amongst the worst offenders in this regard. Many thanks again for tackling high-priority PR middle-end/18628. Roger -- -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=18628
