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

Reply via email to