This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: try_merge_delay_insn with delay list > 1

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 :)



-----Message d'origine-----
De : Jeff Law [] 
Envoyé : vendredi 17 avril 2015 18:41
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

Instead of



j = 1

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

foo (argument1,

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

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]