On 19 June 2012 17:08, Steven Bosscher <stevenb....@gmail.com> wrote: > 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. >
It seems that IN_GCC_FRONTEND was never defined throughout. I did not realise this. Have corrected it now in Make-lang.in and removed all code that depended on it. > 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.) > > I tried switching, but unfortunately it broke the code generation of D thunks. D requires any class that inherits from an interface to output thunks for that interface regardless of how far back it is and whether it being external to the current module. However, the GCC backend as far as I can tell only outputs aliases when the function is output as well. So if there is no function to output no thunk will be generated. This leaves a handful of undefined references to thunks that were defined in the D frontend, but discarded by the backend. Whereas forcing the output using output_mi_thunk means the thunk is emitted and does not cause issues. So removing output.h will be a bit of a problem for me with no obvious way around it. > 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. > > Most functions are of the same name but from different classes, eg: toElem, toIR, toCtype, toObjFile are a few of the main functions which cover the GCC code generation of all expressions and statements passed from the D frontend. > 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. > After some discussion, I have remove the D Inline Asm implementation completely from GDC, along with the backend headers that it depended on, so this file is no more, along with the i386 special cases. > > 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. > Thanks for the tip! Have switched it over. -- Iain Buclaw *(p < e ? p++ : p) = (c & 0x0f) + '0';
