On 3/27/19 8:36 AM, Jakub Jelinek wrote: > On Sun, Mar 24, 2019 at 09:20:07AM -0600, Jeff Law wrote: >> However, I'm increasingly of the opinion that MIPS targets need to drop >> off the priority platform list. Given the trajectory I see for MIPS >> based processors in industry, it's really hard to justify spending this >> much time on them, particularly for low priority code quality issues. > > Besides what has been discussed on IRC for the PR89826 fix, that we really > need a df_analyze before processing the first block, because otherwise we > can't rely on the REG_UNUSED notes in the IL, I see some other issues, but I > admit I don't know much about df nor regcprop. RIght. I plan to commit that today along with the test reordering you pointed out.
> > 1) the df_analyze () after every (successful) processing of a basic block > is IMHO way too expensive, I would be very surprised if df_analyze () isn't > quadratic in number of basic blocks and so one could construct testcases > with millions of basic blocks and at least one regcprop change in each bb > and get at cubic complexity (correct me if I'm wrong, and I'm aware of the > 95% bbs you said won't have any changes at all) I'm going to look this further today. > > 2) another thing I've noticed today, we queue up the debug insn changes, > but if we iterate the second time for any bb, we overwrite and thus lose any > of the debug insn queued changes from the first iteration, so those changes > aren't applied (wrong-debug?) This is actually what worries me, both with the current bits and with any bits that change to a worklist of blocks to reprocess. As is I think we preserve the debug stuff across the iterations which should keep debug info correct. Managing that in a world where we queue up the iteration step is going to be tougher. Queuing the iteration step feels like gcc-10 to me, but we'll see if it falls out easier than expected. > > 3) as mentioned on IRC, the order of tests in copyprop_hardreg_forward_1 > conditional doesn't start from the cheapest to most expensive one That's going to be fixed in today's commit since it's been tested. Jeff