Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable /OPT:ICF on windows, it merges identical functions breaking i… #49622

Merged
merged 1 commit into from
Mar 15, 2021

Conversation

vargaz
Copy link
Contributor

@vargaz vargaz commented Mar 15, 2021

…call symbol lookup.

@lewing
Copy link
Member

lewing commented Mar 15, 2021

fixes #49588

@lewing
Copy link
Member

lewing commented Mar 15, 2021

runtime (Libraries Test Run release mono Linux x64 Debug) failure is #49583

@jaykrell
Copy link
Contributor

  1. What is the size cost?
  2. How about you make the afflicted functions not identical? For example, give them some static data that they use in a pointless way.
  3. Can it be made to work despite the functions being identical?

gcc has an option or attribute to consider address-taken functions not unique, and avoid folding them.
This would fix that. But Visual C++ doesn't have that feature. Alas.

I think SQL Server has this very same problem as Mono, and also disables ICF to workaround it. Alas.

@jaykrell
Copy link
Contributor

@lewing
Copy link
Member

lewing commented Mar 15, 2021

runtime (Libraries Test Run release mono windows x64 Debug) is #49569

@lewing
Copy link
Member

lewing commented Mar 15, 2021

  1. What is the size cost?
  2. How about you make the afflicted functions not identical? For example, give them some static data that they use in a pointless way.
  3. Can it be made to work despite the functions being identical?

gcc has an option or attribute to consider address-taken functions not unique, and avoid folding them.
This would fix that. But Visual C++ doesn't have that feature. Alas.

I think SQL Server has this very same problem as Mono, and also disables ICF to workaround it. Alas.

It is a correctness fix with schedule impact so I'm landing it now but I opened an issue to reexamine the optimization before release.

@jaykrell
Copy link
Contributor

Understood and I expect no relief upstream but couldn't ignore it, sorry.

@AntonLapounov
Copy link
Member

AntonLapounov commented Mar 15, 2021

Note that in VM we use the FCUnique macro to prevent some otherwise identical functions from folding. See #39810 for an example.

@ghost ghost locked as resolved and limited conversation to collaborators Apr 14, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants