Re: [SH4] splits after reload

["Rakesh Kumar - Software, Noida" <rakeshku at noida dot hcltech dot com>]

Hi,

Thank you for your valuable time you have given to study these
problems.

> Is a define_peephole allowed to modify or delete instructions that it
> did not match?  I haven't found anything to this effect in md.texi.
> If there isn't some clause there that I overlooked, we should probably
> add one before we require this to work.

No, md.texi does not mention it.

> When a pattern is unnamed, we generally use the previous name with +n
> appended, where n is a number describing how far back the 
> named pattern is.
> (E.g. if there are four splitter patterns after a named 
> pattern extendpqiti2,
> the third splitter is called extendpqiti2+3.)

Sorry for the mistake. 

> The constant 512 as a hard upper limit on the number of 
> handled instructions
> seems rather arbitrary.  I think you should use a varray 
> instead, growing
> it as needed. (e.g. you can start with 512 and double every 
> time you run out
> of space).

Agreed. The patch is just for taking views on the strategy
part. I had this on my cards in final version.

> (const_int 0) is the NOP pattern.  This will generate an actual NOP
> instruction, and can't be optimized away unless it is unreachable.
> Instead, you can try to use DONE to emit nothing (I haven't checked if
> this is currently supported, but if not, it shouldn't be hard 
> to fix that),
> or emit a nop-op move.

I'd take care of that.

> I get the impression that fix_df_splits is called possibly 
> multiple times,
> and will fail if free (0) fails.

fix_df_splits would be called only one time, since there is
only marker (one UNSPECV_DF_SPLIT) to which peep2 matches.

> I also think that it is rather fragile to store information about the
> interesting instruction at split time, and the 
> UNSPECV_DF_SPLIT might get
> in the way of valid optimizations.

I'm storing the instructions in flow2 and manipulating them
in peephole2. So, the optimizations coming in way are optimizations
applied in flow2. AFAIK, UNSPECV could be a problem in flow2 (in
mark_used_regs), but that can be avoided by using UNSPEC instead. What
we need is just a marker for peep2 to identify. More or less, I agree to
you. But that should be fixable.
 
> You are also calling delete_insn in try_recombine; this is asking for
> trouble if the deleted instruction happens to precede the instruction
> that is matched by the peephole2.

I've taken care of that. The preceding insn would not be deleted.
 
> Considering that you habe O(n^2) behaviour anyway - you scan the insn
> stream in the current extended basic block for every 
> increment - I think
> you might as well just match an increment with a peephole2, and search
> forward for a matching increment.

There is a difference in both strategies. The cause of the problem are
address arithmetic insns generated in DFmode splits. So only those blocks
are scanned which contain these insns. For ADD #4, rn; that scan is
backward.
There is no point in scanning forward in this case; since rn would be
mentioned in the very next insn, as per behavior of define_split for DFmode.
Similary, for ADD #-4, rn; the scan is in forward direction. Therefore,
although being exhibiting O(n^2) behavior, it is less expensive.

On the other hand, a peephole for every increment would scan every block
having increment multiple times. Those increments may be present in the
instruction stream before flow2 and hence are not potential candidates for
recombination. However, it has advantages. It would be free from the
hazards of UNSPECV or UNSPEC, whatever marker we choose. Also, time would
not be spent on checking whether the recorded insn is live after flow2. I'm
trying on this strategy.

In both cases, md.texi needs to be modified.

Thanks for your suggestions. These helped........

Regards,
Rakesh Kumar