On 06/05/2017 04:40 PM, Eric Botcazou wrote: > This is a regression present on the 6 branch for MIPS (and latent on the 7 > branch and mainline). The reorg pass generates wrong code for the attached > testcase with a combination of options under the form of a double lo_sum(sym) > instruction for a single hi(sum). > > The pass starts from a convoluted CFG with 4 instances of the same pair: > > set $16,hi(sym) (1) > set $16,lo_sum($16,sym) (2) > > and generates wrong code after 12 steps: first, the 4 (1) instructions are > moved into delay slots, then one of them is stolen and moved into another > slot, then a second one is deleted, then a third one is also stolen, and then > the first stolen one is deleted. Then 3 (2) instructions are moved into the > same delay slots (again empty) as the (1) and, finally, one of them is stolen. > > All this dance considerably changes the live ranges of the $16 register, in > particular the deletion of 2 (1) instructions. The strategy of reorg is not > to update the liveness info, but to leave markers instead so that it can be > recomputed locally. But there is a hitch: it doesn't leave a marker when > > /* Ignore if this was in a delay slot and it came from the target of > a branch. */ > if (INSN_FROM_TARGET_P (insn)) > return; > > This exception has been there since the dawn of time and I can guess what > kind > of reasoning led to it, but it's probably valid only for simple situations > and > not for the kind of big transformations described above. > > Lifting it fixes the wrong code because it leaves the necessary markers when > instructions that were stolen are then deleted. Surprisingly(?) enough, it > doesn't seem to have much effect outside this case (e.g. 0 changes for the > entire compile.exp testsuite at -O2 on SPARC and virtually same cc1 binaries). > > Tested on SPARC/Solaris, objections to applying it on mainline,7 & 6 branches? > > > 2017-06-06 Eric Botcazou <ebotca...@adacore.com> > > PR rtl-optimization/80474 > * reorg.c (update_block): Do not ignore instructions in a delay slot. > I'll trust your judgement on this one... The updating parts of reorg.c were always IMHO sketchy and anything which brings more consistency to the update mechanism is a step forward -- and IMHO this patch fits that category.
jeff
