This is the mail archive of the
mailing list for the GCC project.
Re: try_merge_delay_insn with delay list > 1
- From: Jeff Law <law at redhat dot com>
- To: BELBACHIR Selim <selim dot belbachir at fr dot thalesgroup dot com>, "gcc at gcc dot gnu dot org" <gcc at gcc dot gnu dot org>
- Date: Fri, 17 Apr 2015 10:40:42 -0600
- Subject: Re: try_merge_delay_insn with delay list > 1
- Authentication-results: sourceware.org; auth=none
- References: <9C88BF562A27AA41B242B2780441926E220C498E67 at THSONEA01CMS05P dot one dot grp> <aebc1095-3cfe-4d6f-b006-6ab47307fef7 at THSONEA01HUB02P dot one dot grp>
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
j = 1
Lines should be wrapped at 80 columns. So you end up with something
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.