Re: [PATCH, 2/2][nvptx, PR83589] Workaround for branch-around-nothing JIT bug

[Tom de Vries <Tom_deVries at mentor dot com>]

On 01/24/2018 03:07 PM, Jakub Jelinek wrote:
> On Wed, Jan 24, 2018 at 02:56:28PM +0100, Tom de Vries wrote:
> > +#if WORKAROUND_PTXJIT_BUG_2
> > +/* Variant of pc_set that only requires JUMP_P (INSN) if STRICT.  This variant
> > +   is needed in the nvptx target because the branches generated for
> > +   parititioning are NONJUMP_INSN_P, not JUMP_P.  */
> > +
> > +static rtx
> > +nvptx_pc_set (const rtx_insn *insn, bool strict = true)
> > +{
> > +  rtx pat;
> > +  if ((strict && !JUMP_P (insn))
> > +      || (!strict && !INSN_P (insn)))
> > +    return NULL_RTX;
> > +  pat = PATTERN (insn);
> > +
> > +  /* The set is allowed to appear either as the insn pattern or
> > +     the first set in a PARALLEL.  */
> > +  if (GET_CODE (pat) == PARALLEL)
> > +    pat = XVECEXP (pat, 0, 0);
>
> This could have been single_set.

This is just a copy of pc_set in jump.c, with the strict parameter added.

It's possible that we can use single_set in pc_set in jump.c. But there 
are subtle differences:
- current pc_set allows a second non-dead set in parallel
- single_set doesn't allow second non-dead set in parallel

I don't know whether this difference is significant or not.

> > +  if (!x)
> > +    return NULL_RTX;
> > +  x = SET_SRC (x);
> > +  if (GET_CODE (x) == LABEL_REF)
> > +    return x;
> > +  if (GET_CODE (x) != IF_THEN_ELSE)
> > +    return NULL_RTX;
> > +  if (XEXP (x, 2) == pc_rtx && GET_CODE (XEXP (x, 1)) == LABEL_REF)
> > +    return XEXP (x, 1);
> > +  if (XEXP (x, 1) == pc_rtx && GET_CODE (XEXP (x, 2)) == LABEL_REF)
> > +    return XEXP (x, 2);
> > +  return NULL_RTX;
>
> And this looks like condjump_label.  

This is just a copy of condjump_label in jump.c, with the strict 
parameter added.

> What are the nvptx conditional jumps
> that aren't JUMP_INSN and why?  That looks like a bad idea.

OpenACC has different execution modes:
- gang redundant vs gang partitioned
- worker single vs worker partitioned
- vector single vs vector partitioned

The transitions between the different modes are represented by:
- nvptx_fork
- nvptx_forked
- nvptx_join
- nvptx_joined
until pass_machine_reorg.

In pass_machine_reorg, they are expanded into more detailed operations 
implementing state propagation and neutering code for single mode.

The neutering code consists of branch-around code, which uses these 
conditional jumps that are not JUMP_INSN.

My assumption is that this is done in order to make the compiler behave 
conservatively with these jumps.  I'm not sure if this is related to one 
or more passes after reorg, or if this is just defensive programming.

I could try to change these into JUMP_INSN in stage1, and see how that goes.

> Otherwise, there is also JUMP_LABEL (insn)...

Right, that one requires a JUMP_INSN.

Thanks,
- Tom