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.
