On 1/23/19 12:29 PM, Segher Boessenkool wrote: > On Wed, Jan 23, 2019 at 04:52:24PM +0300, Alexander Monakov wrote: >> On Wed, 23 Jan 2019, Andrey Belevantsev wrote: >>> For that, I'm not sure. Your patch will leave the tablejump unscheduled at >>> all, i.e. any fields like INSN_TICK would be unfilled and thus the later >>> passes like bundling on ia64 will not work. Maybe we can just stop >>> tablejumps from moving within its block, Alexander? >> >> On the Bugzilla there's an alternative patch by Segher that adjusts the >> assert >> to accept this situation (there's still a barrier insn after the tablejump >> insn, >> it's just after a jump_table_data insn rather than immediately following the >> jump). That should be a better approach, and David said he was testing it. >> >> That said, I'm really concerned that on this testcase we should not be moving >> the tablejump *at all*: we are moving it up past a 'use ax' insn (the use is >> for the function's return value). So after the move the use is in an >> unreachable >> block, which makes no sense. > > Yes, that makes no sense indeed. Is it acceptable (for performance) to > simply bail out on table jumps here? > >> So my concern is that just fixing the assert changes the issue from the ICE >> to a >> (latent) wrong-code issue. >> >> There should have been an anti-dependence between the use and the jump. I'll >> try >> to investigate. > > Or that. Thanks! Or (as I've indicated to David) investigate if we're getting SCHED_GROUP_P right -- that's the canonical way to keep two insns together in the schedule. It's definitely worth investigating.
Jeff