This is the mail archive of the gcc-patches@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: how can I add gcc backend code into GCC package


> 1: We have finished applying process in FSF.
> 2: We are new comer, never propose the port to the GCC before. and our 
> 32bits S+core CPU has its owns independent ISA.
> 3: We port S+core GCC backend for GCC-4.1.1 release version and passed all 
> testsuit, now update it to current mainline development trunk of GCC. Can 
> you kindly add this patch to the mainline development trunk of GCC.
> 4: Other toolchains for S+core(like binutils, gdb/sim) are also applying 
> in process.

	The GCC Steering Committee has agreed to accept the contribution
of the S+Core port.  The patch itself needs to be approved by a maintainer
with authority for areas of the compiler covering ports, such as a Global
Write Privileges or a Middle-End maintainer.

	I looked over the patch and while I do not know the details of the
S+Core processor, the general technical design of the port looks okay.  If
it works and produces satisfactory code, there is no need to second-guess
your decisions.

	For instance, I am a little surprised that the MD file utilizes
define_expand so rarely, e.g., movsi is implemented as define_insn.

	Also, why do you define

+#define BIT_PER_WORD				32

instead of using BITS_PER_WORD?

	I think the main problem with the patch is formatting: extra
spaces, mixing spaces and tabs, wrong comment format, inconsistent
format.

	For instance,

+#define TARGET_DEFAULT	\
+  ( MASK_SCORE7)
    ^--- extra space


+       .frame  r0,0,r3, 0
+    .mask   0x00000000,0

switching between tab and space?

+/**
+ * Passing Arguments in Registers 
+ */

Wrong comment format.

+/* F_D_BITOFF is the number of bits offset between the MSB of the mantissa
+   of a float and of a double. Assumes there are only two float types.
+   (double::FRAC_BITS+double::NGARDS-(float::FRAC_BITS-float::NGARDS))
+ */

Again comment format.  Period, two spaces, and end comment.

+  "@bittst!  %0, %c1
+    bittst.c %0, %c1"

The '@' and output templates should be on separate lines, or at least
consistent throughout the port.

+(define_insn_and_split "*truncate_qi_ucc"
+  [(set (reg:CC_NZ CC_REGNUM)
+            (compare:CC_NZ 
+					(and:SI (match_operand:SI 1 "register_operand"  "d")
+							(match_operand:SI 2 "const_int_operand" ""))
+                (const_int 0)))

	Weird indentation in MD file.  The operands and operators in the
patterns should line up.

+(define_insn "popsi"
+  [(set (match_operand:SI 0 "register_operand" "=d")
+	 (match_operand:SI 1 "pop_operand"	   " >"))]
+  ""
+  " pop!   %0, [r0]"
+  [(set_attr "type" "store")
+   (set_attr "mode" "SI")])

+(define_insn "movhilo"
+	      [(parallel [(set (match_operand:SI 0 "register_operand" "=d")
+			 (match_operand:SI 1 "loreg_operand"	""))
+					       (set (match_operand:SI 2 "register_operand" "=d")
+				 (match_operand:SI 3 "hireg_operand" ""))])]
+	""
+	"mfcehl	%2, %0"
+ [(set_attr "type" "fce")
+  (set_attr "mode" "SI")])

(define_insn "movsicc_internal"
+  [(set (match_operand:SI 0 "register_operand" "=d")
+  (if_then_else:SI 
+			(match_operator 1 "comparison_operator" 
+								[(reg:CC CC_REGNUM)
+				 (const_int 0)])
+						(match_operand:SI 2 "arith_operand" "d")
+				(match_operand:SI 3 "arith_operand" "0")))]
+  ""
+  "mv%C1	%0, %2"
+  [(set_attr "type"	"cndmv")
+   (set_attr "mode"	"SI")])

You should be consistent about the indentatation of the patterns, output
templates, and the spacing within instruction output template strings.
The code output to the assembly file will jump around.

	Also, misc.md appears to use spaces white score.md uses '\t'.

+  if (!CONST_OK_FOR_LETTER_P (offset, 'O')){
+    reg = expand_simple_binop (GET_MODE (reg), PLUS,
+	          GEN_INT (offset & 0xffffc000),
+				         reg, NULL, 0, OPTAB_WIDEN);

	And there are places where braces are not on a new line with the
code properly indented.

	I realize that you mainly are focussed on functionality and
correctness, but all code in GCC needs to follow the GNU Coding Standard
and GCC extensions.

	If you corrected the formatting issues, I would recommend approval
of the patch.

David


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