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

win32/Makefile: update compilation flags for Visual C++ #22664

Closed
wants to merge 1 commit into from

Conversation

xenu
Copy link
Member

@xenu xenu commented Oct 14, 2024

Makefile contains the following comment from 2002:

# -O1 yields smaller code, which turns out to be faster than -O2

Visual C++ documentation says that -O2 "creates fast code", while -O1 "creates small code". It's unlikely that, after 20 years, -O2 still generates worse code. If it does, it should be reported to Microsoft.

Also added -Gw (optimize global data).

Makefile contains the following comment from 2002:

  # -O1 yields smaller code, which turns out to be faster than -O2

Visual C++ documentation says that -O2 "creates fast code", while -O1
"creates small code". It's unlikely that, after 20 years, -O2 still
generates worse code. If it does, it should be reported to Microsoft.

Also added -Gw (optimize global data).
@bulk88
Copy link
Contributor

bulk88 commented Oct 14, 2024

I disagree with -O2, extra code growth every where. Our Perl_croak()/Perl_die() and "panic: " type fatal error branches, will be INLINED EVERYWHERE if you do that.

-Gw I def agree, it cut 70 bodies_by_type copies to 50 when I turned it on. Instead of -O2, add -Oi https://learn.microsoft.com/en-us/cpp/build/reference/oi-generate-intrinsic-functions?view=msvc-170

Inlined memcmp(ptr, "exit", 4) is push, push, push call, clean c stack, right now, -Oi makes that 1 CPU op, I shoudve added -Oi years ago to core.

@bulk88
Copy link
Contributor

bulk88 commented Oct 14, 2024

I disagree with -O2, extra code growth every where. Our Perl_croak()/Perl_die() and "panic: " type fatal error branches, will be INLINED EVERYWHERE if you do that.

-Gw I def agree, it cut 70 bodies_by_type copies to 50 when I turned it on. Instead of -O2, add -Oi https://learn.microsoft.com/en-us/cpp/build/reference/oi-generate-intrinsic-functions?view=msvc-170

Inlined memcmp(ptr, "exit", 4) is push, push, push call, clean c stack, right now, -Oi makes that 1 CPU op, I shoudve added -Oi years ago to core.

__forceinline macro support exists in P5 Core headers already, and that overrides all inline "cost analysis" by MSVC even on -O1, If there is a very very good justification, in very limited situations, just use the proper perl macro that already exists. Plus if you change -O1 to -O2, you are affecting all of CPAN.

edit: -O2 has a terrible side effect, that EVERY CONSTANT in machine code is extended to 4 bytes.

char ch = 0x1"

under -O1 that is 3 bytes of machine code, under MSVC -O2, that becomes 7 bytes every time.

@xenu
Copy link
Member Author

xenu commented Oct 14, 2024

I don't have the energy to argue with you.

I did this to partially address your complaints about the lack of inlining, as this change actually helps with e.g. newSV(iv|nv).

If you don't want it, that's fine by me. I don't care about Visual C++. Almost none of our users use it. Both Strawberry and ActivePerl are built using MinGW.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants