This is the mail archive of the gcc@gcc.gnu.org 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


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.



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