I've attached the fixed version of the patch. I've tested it on the trunk with 
my private target.
I can't provide a test because apparently no backend (other than my private 
one) uses delay slots with more that 1 slot.
I was also unable to test the behaviour of this patch for an hypothetic target 
providing delay lots with more that 1 slot AND the possibility to annul 
instruction in delay slots.

It seems to me that this patch is a small enhancement anyway.
I hope it's ok for trunk :)

Regards,

Selim

-----Message d'origine-----
De : Jeff Law [mailto:l...@redhat.com] 
Envoyé : vendredi 17 avril 2015 18:41
À : BELBACHIR Selim; gcc@gcc.gnu.org
Objet : Re: try_merge_delay_insn with delay list > 1

On 03/10/2015 07:40 AM, BELBACHIR Selim wrote:
> Me again :)
>
> I enhanced my patch because it was not generalized for instructions with N 
> delay_slots.
Mostly OK, though there are some formatting nits that need to be corrected.

We have whitespace around arithmetic, logical and comparison operators to 
separate them from their operands.  So instead of

slot_number+j

Use

slot_number + j

Instead of

j=1

Use

j = 1

Lines should be wrapped at 80 columns.  So you end up with something like this

foo (argument1,
      argument2,
      argument3)

ie, when you wrap, the arguments to the call will line up vertically.

It may help wrapping to create a local variable to hold PATTERN (insn). 
  Call it 'pat' :-)

When referring to variables or parameters in a comment, capitalize them.

The patch may need updating for the trunk, please test it with the trunk when 
you ask  for it to be included on the trunk.

These are all fairly minor issues.  The actual change seems reasonable.

What systems do you have that you could do a bootstrap and regression test 
with?  Ideally since you're changing the delay slot branching code it'd be a 
system with delay slots :-)  If that's not possible because you don't have 
access to a bootstrapping system with delay slots, make sure to mention it.

Ideally you'd have a test for this bug. However, with a private port I wouldn't 
consider it a necessity.  However, you may want to go ahead and create one for 
internal uses.  And if you ever submit your port to the offficial sources, you 
can include target specific tests at that time.


Attachment: try_merge_patch3
Description: try_merge_patch3

Reply via email to