On Thu, Nov 22, 2012 at 9:22 PM, Joern Rennecke wrote:
(nothing but a ChangeLog :-)
Looking at the ARC port a bit, and IMHO it doesn't look very messy...
First some general comments:
This target apparently wants to play tricks with reload (e.g.
arc_preserve_reload_p). IMHO at this point ports that don't work with
LRA should not be accepted.
TARGET_EXPAND_ADDDI is not covered in any test. Actually most target
options are not covered.
Can't the following option be just removed:
mold-di-patterns
Target Var(TARGET_OLD_DI_PATTERNS)
enable use of old DI patterns that have presumably been obsoleted by
subreg lowering.
There quite some old cruft that should be cleaned up. A few examples:
1. Things that probably worked before GCC3 but shouldn't be in a new port:
/* /\* Compute LOG_LINKS. *\/ */
/* for (bb = 0; bb < current_nr_blocks; bb++) */
/* compute_block_backward_dependences (bb); */
2. Old scheduler description:
;; Function units of the ARC
;; (define_function_unit {name} {num-units} {n-users} {test}
;; {ready-delay} {issue-delay} [{conflict-list}])
(etc...)
3. Presumably no longer applicable comments if this port is supposed
to be part of the FSF repo:
;; *** N.B.: the use of '* return...' in "*movsi_insn", et al, rely
;; on ARC-local changes to genoutput.c's process_template() function
Some random observations in config/arc/arc.c:
arc_dead_or_set_postreload_p() and the related functions is not used AFAICT.
Many functions (most?) have no leading comments.
About config/arc/arc.md:
What's this comment about?
;; ashwin : include options.h from build dir
;; (include "arc.c")
The is_NON_SIBCALL attribute appears to be unused. If it can be
removed, then so can the is_SIBCALL attribute.
How is the "*millicode_sibthunk_ld" pattern used?
What are the commented-out patterns (e.g. "*adc_0") for?
Before the sibcall and return_i patterns:
; Since the demise of REG_N_SETS, it is no longer possible to find out
; in the prologue / epilogue expanders how many times blink is set.
I don't understand this, REG_N_SETS still exists.
C-style comment in a .md file (does that even work??):
/* Store a value to directly to memory. The location might also be cached.
Since the cached copy can cause a write-back at unpredictable times,
we first write cached, then we write uncached. */
Why are there patterns that have a "switch (which_alternative)" to
emit the insn template, instead of an "@..." string? See e.g.
(define_insn "*zero_extendhisi2_i" and "*zero_extendqisi2_ac".
It seems to me that a bit of cleaning up is in order before this port
is accepted.
Ciao!
Steven
