On Thu, 6 Jul 2017, Matthew Fortune wrote:

> >  Nothing wrong with your proposed change, but overall I wonder if (as a
> > follow-up change) we could find a nonintrusive way to have this pattern
> > (and `clear_hazard_<mode>' as well) produce JRS.HB rather than JR.HB in
> > microMIPS compilations, as using the 32-bit delay-slot NOP encoding
> > where the 16-bit one would do is obviously a tiny, but completely
> > unnecessary code space loss (and we do care about code space losses in
> > microMIPS compilations; conserving space is the very purpose of the
> > microMIPS ISA after all).
> > 
> >  Of course it wouldn't do if we rewrote the instruction pattern as
> > "%(jr%!.hb\t$31%/%)" here, because the NOP that follows would have to
> > come from an RTL instruction for `%!' to have any effect.  But perhaps
> > we could emit RTL instead somehow rather than hardcoding the NOP with
> > `%/'?
> 
> I think this case is so specialist we can safely just switch to writing
> out the NOP directly in the output pattern just keeping the %(%) for
> noreorder. This code will have to be reworked with microMIPSR6 when
> submitted so it can be handled then; good spot to use jrs.hb.

 It does not matter for `%!' whether you use `%/' or spell out `nop' 
literally.  I was more concerned about getting the instruction count 
correctly, which would be 1.5 for the JRS.HB case, however I think you can 
just set the `length' attribute directly, to 6.

 Still the issue of having separate almost identical patterns remains, as 
barring the use of `%!' I think you'll need to qualify them with 
TARGET_MICROMIPS and !TARGET_MICROMIPS respectively, to have different 
instruction mnemonics.  In this case I think you could write (untested):

(define_insn "mips_hb_return_internal"
  [(return)
   (unspec_volatile [(match_operand 0 "pmode_register_operand" ",")]
               UNSPEC_JRHB)]
  ""
  "@
   %(jrs.hb\t$31%/%)
   %(jr.hb\t$31%/%)"
  [(set_attr "compression" "micromips,*")
   (set_attr "length" "6,8")])

however the equivalent for `clear_hazard_<mode>' would be rather horrible 
(OTOH eventually it should use ADDIUPC in its SImode microMIPS variant, so 
perhaps this is acceptable as we'll have multiple different sequences 
anyway).

 For microMIPSr6 we'll then just have another variant with no delay slot.

  Maciej

Reply via email to