public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: LIU Hao <lh_mouse@126.com>
Cc: Xi Ruoyao <xry111@xry111.site>,
	GCC Patches <gcc-patches@gcc.gnu.org>, JonY <10walls@gmail.com>
Subject: Re: [PATCH] Remove size limit of PCH
Date: Tue, 10 May 2022 16:28:08 +0200	[thread overview]
Message-ID: <Ynp2dghYz8EX3RAd@tucnak> (raw)
In-Reply-To: <2eaf9cb8-c215-4595-1dca-4261f88fe4e3@126.com>

On Tue, May 10, 2022 at 09:30:03PM +0800, LIU Hao via Gcc-patches wrote:
> From 2e314baed84fd80b3b3c4c67787a032b86dd54dc Mon Sep 17 00:00:00 2001
> From: LIU Hao <lh_mouse@126.com>
> Date: Tue, 10 May 2022 13:19:07 +0800
> Subject: [PATCH] [PR14940] Remove size limit of PCH
> 
> There shouldn't be such a limit in practice.
> 
> 2022-05-03  LIU Hao <lh_mouse@126.com>
> 
> 	PR pch/14940
> 	* config/i386/host-mingw32.cc (pch_VA_max_size): Remove.
> 	(mingw32_gt_pch_get_address): Remove size limit.

This looks reasonable, but doesn't contain the most important part.
As mentioned in https://gcc.gnu.org/r12-5855
the generic part can now support relocation of PCH, so it is fine if it is
mapped at some other address, it is preferrable if it is mapped at the same
address as it was written for, but if not, the generic code can relocate it.

So, beyond your changes, I'd suggest to change:
  /* Retry five times, as here might occure a race with multiple gcc's
     instances at same time.  */
  for (r = 0; r < 5; r++)
   {
      mmap_addr = MapViewOfFileEx (mmap_handle, FILE_MAP_COPY, 0, offset,
                                   size, addr);
      if (mmap_addr == addr)
        break;
      if (r != 4)
        Sleep (500);
   }

  if (mmap_addr != addr)
    {
      w32_error (__FUNCTION__, __FILE__, __LINE__, "MapViewOfFileEx");
      CloseHandle(mmap_handle);
      return  -1;
    }

IMHO you can just drop the loop, sleep of half a second is almost certainly
slower than the relocation handling, so I'd just
  mmap_addr = MapViewOfFileEx (mmap_handle, FILE_MAP_COPY, 0, offset,
			       size, addr);
  if (mmap_addr == NULL)
    {
      w32_error (__FUNCTION__, __FILE__, __LINE__, "MapViewOfFileEx");
      CloseHandle(mmap_handle);
      return -1;
    }

  addr = mmap_addr;

This, if it mmaps the file at the right address, nice, if not, let the
caller know (through updating addr) that it needs to relocate it,
but if the mapping failed, fail.

	Jakub


  reply	other threads:[~2022-05-11 11:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-10 11:35 LIU Hao
2022-05-10 12:00 ` Xi Ruoyao
2022-05-10 13:30   ` LIU Hao
2022-05-10 14:28     ` Jakub Jelinek [this message]
2022-05-11 14:31       ` LIU Hao
2022-05-11 11:44     ` Jonathan Yong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Ynp2dghYz8EX3RAd@tucnak \
    --to=jakub@redhat.com \
    --cc=10walls@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=lh_mouse@126.com \
    --cc=xry111@xry111.site \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).