This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Ping^6: contribute Synopsys Designware ARC port
- From: Diego Novillo <dnovillo at google dot com>
- To: Joern Rennecke <joern dot rennecke at embecosm dot com>
- Cc: gcc-patches <gcc-patches at gcc dot gnu dot org>, Richard Earnshaw <richard dot earnshaw at arm dot com>, Richard Biener <rguenther at suse dot de>, Richard Henderson <rth at redhat dot com>, Jakub Jelinek <jakub at redhat dot com>, Geoffrey Keating <geoffk at geoffk dot org>, Richard Kenner <kenner at nyu dot edu>, Jeff Law <law at redhat dot com>, Michael Meissner <gnu at the-meissners dot org>, Jason Merrill <jason at redhat dot com>, "David S. Miller" <davem at redhat dot com>, Mark Mitchell <mark at codesourcery dot com>, Joseph Myers <joseph at codesourcery dot com>, Bernd Schmidt <bernds at codesourcery dot com>, Ian Lance Taylor <ian at airs dot com>, Jim Wilson <wilson at tuliptree dot org>
- Date: Tue, 1 Oct 2013 13:23:48 -0400
- Subject: Re: Ping^6: contribute Synopsys Designware ARC port
- Authentication-results: sourceware.org; auth=none
- References: <20130928095442 dot dpsqk8x5440k4ko0-nzlynne at webmail dot spamcop dot net>
On Sat, Sep 28, 2013 at 9:54 AM, Joern Rennecke
<joern.rennecke@embecosm.com> wrote:
> The main part of the port (everything but the testsuite) is still waiting
> for review:
> http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00323.html
> http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00324.html
> http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00325.html
> http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00328.html
> http://gcc.gnu.org/ml/gcc-patches/2013-09/msg01870.html
> http://gcc.gnu.org/ml/gcc-patches/2013-09/msg02070.html
I have finished reading through these patches. They are OK to commit.
The changes indicated below are minor. Ideally, you'd address them
before committing the patch, but if it's easier to do it post-commit,
that's OK too.
- The Copyright years should be 2013 in every new file. Or has this
port been released before?
- In config/arc/arc-protos.h:
+/* insn-attrtab.c doesn't include reload.h, which declares
regno_clobbered_p. */
+extern int regno_clobbered_p (unsigned int, rtx, enum machine_mode, int);
Why not include reload.h here? Interface changes (however rare) make
this a hassle.
- In config/arc/simdext.md
+;; Va, [Ib,u8] instructions
+;; (define_insn "vld32wh_insn"
+;; [(set (match_operand:V8HI 0 "vector_register_operand" "=v")
+;; (vec_concat:V8HI (unspec:V4HI [(match_operand:SI 1 "immediate_operand" "P")
+;; (vec_select:HI (match_operand:V8HI 2 "vector_register_operand" "v")
+;; (parallel [(match_operand:SI 3 "immediate_operand" "L")]))]
UNSPEC_ARC_SIMD_VLD32WH)
+;; (vec_select:V4HI (match_dup 0)
Necessary? If so, please add a comment stating why it's commented out.
- In doc/extend.texi:
+Permissible values for this parameter are: @w{@code{ilink1}} and
+@w{@code{ilink2}}.
+
ARC developers already know what ilink1 and ilink2 mean?
+@cindex indirect calls on Epiphany
+These attribute specifies how a particular function is called on
+ARC, ARM and Epiphany
s/specifies/specify/
+because __alignof__ sees only the type of the dereference, wheras
+__builtin_arc_align uses alignment information from the pointer
s/wheras/whereas/
- I have not fully cross-referenced the list of documented builtins vs
the list of implemented builtins. Please double check them.
- Ditto the list of -m options. It looks like they're all documented,
but I haven't diff'd the doc vs the options file.
- In libgcc/config/arc/gmon/mcount.c
The file has a different copyright/license notice at the top. Is this
from a third party source? Can it be changed to lgpl?
+#if 0
+ if (catomic_compare_and_exchange_bool_acq (&p->state, GMON_PROF_BUSY,
+ GMON_PROF_ON))
+ return;
Get rid of this?
+#elif defined (__ARC700__)
+/* ??? This could temporrarily loose the ERROR / OFF condition in a race,
s/temporrarily/temporarily/
s/loose/lose/
- Many files in libgcc/config/arc/... have #if0 blocks. Are they
really necessary?
- In libgcc/config/arc/ieee-754/arc600-dsp/muldf3.S
+/* We have checked for infinitey / NaN input before, and transformed
+ denormalized inputs into normalized inputs. Thus, the worst case
s/infinitey/infinity/
It also happens in another muldf3.S file in a sibling directory.
- libgcc/config/arc/t-arc-newlib does not contain the exception clause.
- In config/arc/arc.md there are several define patterns commented
out. Toss them out?
- In config/arc/arc.c:
No need to include stdio.h
No need to mark struct arc_frame_info with GTY. It contains no pointers.
In arc_expand_epilogue():
Get rid of fp_restored_p
if (1)
{
unsigned int pretend_size = cfun->machine->frame_info.pretend_size;
Just move everything out of the if()?
In output_shift(): Get rid of the #if 1s?
In arc_encode_section_info():
/* Symbols in the text segment can be accessed without indirecting via the
constant pool; it may take an extra binary operation, but this is still
faster than indirecting via memory. Don't do this when not optimizing,
since we won't be calculating al of the offsets necessary to do this
simplification. */
But that seems out of sync with the code. It never checks whether
optimizations are enabled.
Thanks. Diego.