On 1/23/19 6:52 AM, 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.
> 
> 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.
The first thing I'd do is make sure the jump table has its SCHED_GROUP_P
marker which indicates it must stick with its associated jump.  Then I'd
make sure it's honored in the appropriate places.  It may be awkward
because the jump table data insn is going to have the SCHED_GROUP_P bit
set, but we're likely looking to muck with the actual jump.  That's
probably an artifact of the old backwards scheduling algorithm.

jeff

Reply via email to