[PATCH, gdc] - Merging gdc (GNU D Compiler) into gcc

Steven Bosscher stevenb.gcc@gmail.com
Tue Jun 19 16:27:00 GMT 2012


Hello,

I had a very quick look through the gdc_frontend patch. Below are a
couple of comments on it:

> http://www.gdcproject.org/files/gdc_frontend.patch.gz
>
> [PATCH 1/4]:
> The D compiler frontend
>  -  gcc/d

How did you test this? You include rtl.h/expr.h in d-builtins.c and
d-gcc-includes.h, which should both be in ALL_HOST_FRONTEND_OBJS and
fail to build because IN_GCC_FRONTEND is defined and GCC_RTL_H is
poisoned. See system.h:

/* Front ends should never have to include middle-end headers.  Enforce
   this by poisoning the header double-include protection defines.  */
#ifdef IN_GCC_FRONTEND
#pragma GCC poison GCC_RTL_H GCC_EXCEPT_H GCC_EXPR_H
#endif

Do you somehow bypass the normal build system? Or maybe you don't
include system.h? Either way, front ends should never have to include
RTL headers.

BTW you also include output.h in those two files, and I am about two
patches away from adding output.h to the list of headers that no front
end should ever include (a front end should never have to write out
assembly). Can you please check what you need output.h for, and fix
this?


What are you calling targetm.asm_out.output_mi_thunk and
targetm.asm_out.generate_internal_label for? Thunks and aliases should
go through cgraphunit.

(NB: This also means that this front end cannot work with LTO. IMHO we
shouldn't let in new front ends that don't work with LTO.)


Many functions have no leading comment, and other GNU coding standard
requirements are not followed either. Those should IMHO be fixed also,
before this front end can be accepted.


There is this comment:
+/* GCC does not support jumps from asm statements.

This isn't really true anymore, as your patch also notes:
+   ------------------------------
+   %% Fix for GCC-4.5+
+   GCC now accepts a 5th operand, ASM_LABELS.
(...)
+   For prior versions of gcc, this requires a backpatch.

It seems to me that if this front end is contributed, handling of
prior version of gcc isn't necessary anymore - that code should just
be removed.


+
+           case Op_de:
+#ifndef TARGET_80387
+#define XFmode TFmode
+#endif
+             mode = XFmode; // not TFmode

What is this hack for? This is not the way to find the right mode for
this operation.

+#ifdef TARGET_80387
+#include "d-asm-i386.h"
+#else
+#define D_NO_INLINE_ASM_AT_ALL
+#endif
+
+/* Apple GCC extends ASM_EXPR to five operands; cannot use build4. */

Idem here. And Apple GCC is irrelevant too, if this front end lands on
FSF trunk.

What is d/d-asm-i386.h for? It looks like i386 is a special case
throughout the front end.


In d-gcc-tree.h:
+// normally include config.h (hconfig.h, tconfig.h?), but that
+// includes things that cause problems, so...
+
+union tree_node;
+typedef union tree_node *tree;

See coretypes.h.

Ciao!
Steven



More information about the Gcc-patches mailing list