Re: [Patch/RFC] Enable PCH for mingw32

[Geoff Keating <geoffk at geoffk dot org>]

Danny Smith <dannysmith@clear.net.nz> writes:

> 1) asm_out_file needs to be opened in binary mode, to match the
> binary mode of the .pch file. This causes no problems at all for
> GNU as.

I think this part is OK.  (The actual problem is that it's read using


> 2) After finishing reading asm_out_file in c-pch.c:c_common_write_pch,
> we need to ensure that the next write operation will be permitted. Either
> a call to a file-positioning function or setting _IOEOF is necessary to
> comply with ISO C.

For this part, the code now looks like:

>     /* asm_out_file can be written afterwards, so must be flushed first.  */
>     fflush (asm_out_file);
> +   /* On some hosts we need to either clear the _IOREAD flag or set _IOEOF.
> +      Otherwise _IOERR will be set on next write.
> +      According ISO C99 (7.19.5.3) spec for fopen() in '+' mode:
> +       "input shall not be directly followed by output without an
> +        intervening call to a file positioning function, unless the input
> +        operation encounters end-of-file."	
> +      This just sets EOF. Alternatively, we could clear _IOREAD as a side effect
> +      of a call to fseek.  */ 
> +   getc(asm_out_file);
> +   if (!feof (asm_out_file))
> +     fatal_error ("incomplete read of %s", asm_file_name);

I suggest replacing both the getc() and the fflush() with fseek---it
seems to me that would be cleaner than a getc() which exists only for
the purpose of reading an EOF.  Also, it looks like the previous
fflush (after the xmalloc) can be deleted, since there's a fseek
there.

> 3) The alignment required for *reserving* address space on win32 hosts
> differs from the alignment for *committing* memory in that space. The
> allocation granularity is also required when mmap'ing a file at an offset.
> I've added a new host macro to allow that to happen.

I think we generally prefer hooks to macros.  Could you make this a new
host hook instead of a new macro?

> 4) HOST_HOOKS_GT_PCH_GET_ADDRESS and HOST_HOOKS_GT_PCH_USE_ADDRESS
> are implemented  using VirtualAlloc and MapViewOfFileEx w32api functions.

This part looked OK (I don't fully understand it, but it looks like I
would expect such code to look).  You may be able to simplify the code a bit
if you notice that mingw32_gt_pch_use_address is called exactly once,
and so the 'if (size == 0)' case can just return immediately.

> The patch causes no new regressions.  Prior to patch all pch testsuites
> failed.  Now I get this with RUNTESTFLAGS=pch.exp
> 
>   === gcc Summary ===
> 
> # of expected passes  875
> # of unexpected failures 7
> # of untested testcases  28
> 
> The failures are all inline-3.c assembly comparisons, of the form:
> line #100
> <  .def _fmod; .scl 3; .type 32; .endef
> > 
> line #101
> <  .def _rint; .scl 3; .type 32; .endef
> > 
> 
>   === g++ Summary ===
> 
> # of expected passes  63
> # of unexpected failures 9
> 
> The failures are assembly comparisons in system-1.C, system-2.C, static-
> 1.C and effectively the same as above, namely the files compiled with
> pch lack the end-of-file list of .def directives for undefined external
> functions.
> 
> I do not believe the failures are critical.. Indeed if I assemble and
> compare dumps of the object files rather than assembly output, there are
> no differences due to use of pch.  
> 
> The list of undefined external functions are output by
> winnt.c:i386_pe_file_end. I do not know how these directives are used by
> GNU as, but they do not seem to be needed. In fact, if I undefine
> ASM_OUTPUT_EXTERNAL (where the external symbols are saved into a list)
> in cygming.h, I can still build working executables and dlls (and the
> pch testsuite failures are resolved). Am I missing something or are
> these end-of-file directives just for compatability with other
> assemblers/linkers as suggested by this comment in winnt.c. 
> 
> /* The Microsoft linker requires that every function be marked as
>    DT_FCN.  When using gas on cygwin, we must emit appropriate .type
>    directives.  */
>  
> 
> Any comments on this would be appreciated.  I have tested only on
> NT4 and XP, so if anybody could test on W9x I would be grateful.

You probably want to fix this by adding appropriate GTY markers in
winnt.c.  I expect that they are needed by some code, otherwise why
would someone have gone to the effort of writing all that code?  Plus,
it's worth it just so that the testsuite passes.


-- 
- Geoffrey Keating <geoffk@geoffk.org>