Compile native code with -ffunction-sections - #79322

Merged
marek-safar merged 3 commits into
mainfrom
MichalStrehovsky-patch-1
Dec 7, 2022
Merged

Compile native code with -ffunction-sections#79322
marek-safar merged 3 commits into
mainfrom
MichalStrehovsky-patch-1

Conversation

@MichalStrehovsky

@MichalStrehovskyMichalStrehovsky commented Dec 7, 2022

Copy link
Copy Markdown
Member

We specify /Gy on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.

Cc @dotnet/ilc-contrib

@MichalStrehovsky
Compile native code with -ffunction-sections
e0b86fc
We specify `/Gy` on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
@ghost

ghost commented Dec 7, 2022

Copy link
Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@jkotas

jkotas commented Dec 7, 2022

Copy link
Copy Markdown
Member

Need to suppress this for tvOS. clang : error : -ffunction-sections is not supported with -fembed-bitcode

@am11

am11 commented Dec 7, 2022

Copy link
Copy Markdown
Member

-ffunction-sections is set for debug and checked builds which is now redundant and can be deleted:

runtime/eng/native/configurecompiler.cmake

Lines 163 to 165 in e0b86fc

# -fdata-sections -ffunction-sections: each function has own section instead of one per .o file (needed for --gc-sections)
# -O1: optimization level used instead of -O0 to avoid compile error "invalid operand for inline asm constraint"
add_compile_options("$<$<OR:$<CONFIG:DEBUG>,$<CONFIG:CHECKED>>:${CLR_SANITIZE_CXX_OPTIONS};-fdata-sections;--ffunction-sections;-O1>")

marek-safar
marek-safar reviewed Dec 7, 2022
View reviewed changes
Comment threadeng/native/configurecompiler.cmake Outdated

# Separate functions so linker can remove them. But not on Apple mobile platforms because
# -ffunction-sections is not supported with -fembed-bitcode.
if (NOT CLR_CMAKE_HOST_TVOS AND NOT CLR_CMAKE_HOST_IOS)

@marek-safarmarek-safarDec 7, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should work for iOS, and only tvOS needs be disabled

@MichalStrehovskyMichalStrehovskyDec 7, 2022

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saw the failed iOS leg and assumed it's the same problem. But iOS maybe failed for unrelated reasons then. I'll put it back:

.packages/microsoft.dotnet.helix.sdk/8.0.0-beta.22579.2/tools/Microsoft.DotNet.Helix.Sdk.MultiQueue.targets(90,5): error : (NETCORE_ENGINEERING_TELEMETRY=Test) Work item System.Runtime.Tests in job 5143df8d-e525-435f-9a37-d062df01b8cb has failed.

@ghost

ghost commented Dec 7, 2022

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

We specify /Gy on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.

Cc @dotnet/ilc-contrib

Author:MichalStrehovsky
Assignees:MichalStrehovsky
Labels:

area-Infrastructure

Milestone:-

filipnavara
filipnavara reviewed Dec 7, 2022
View reviewed changes
Comment threadeng/native/configurecompiler.cmake
Comment on lines +483 to +487
# Separate functions so linker can remove them. But not on tvOS because
# -ffunction-sections is not supported with -fembed-bitcode.
if (NOT CLR_CMAKE_HOST_TVOS)
add_compile_options(-ffunction-sections)
endif()

@filipnavarafilipnavaraDec 7, 2022•
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is tvOS still compiled with bitcode? I thought Apple basically deprecated that across the board.

https://developer.apple.com/documentation/Xcode-Release-Notes/xcode-14-release-notes:

Starting with Xcode 14, bitcode is no longer required for watchOS and tvOS applications, and the App Store no longer accepts bitcode submissions from Xcode 14.

Xcode no longer builds bitcode by default and generates a warning message if a project explicitly enables bitcode: “Building with bitcode is deprecated. Please update your project and/or target settings to disable bitcode.” The capability to build with bitcode will be removed in a future Xcode release. IPAs that contain bitcode will have the bitcode stripped before being submitted to the App Store. Debug symbols can only be downloaded from App Store Connect / TestFlight for existing bitcode submissions and are no longer available for submissions made with Xcode 14. (86118779)

@am11am11Dec 7, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is using v13:

Running 'CC="/Applications/Xcode_13.2.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang"..

@filipnavarafilipnavaraDec 7, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it necessarily matters what CI uses. The change is that App Store doesn't require it anymore for new submissions.

@filipnavarafilipnavaraDec 7, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#79349

@marek-safar
marek-safar merged commit 20af770 into mainDec 7, 2022
@marek-safar
marek-safar deleted the MichalStrehovsky-patch-1 branch December 7, 2022 17:51
@MichalStrehovskyMichalStrehovsky mentioned this pull request Dec 8, 2022
Merged
@ghostghost locked as resolved and limited conversation to collaborators Jan 6, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@marek-safarmarek-safarmarek-safar left review comments
@jkotasjkotasjkotas approved these changes
+2 more reviewers
@filipnavarafilipnavarafilipnavara left review comments
@am11am11am11 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@MichalStrehovskyMichalStrehovsky

Labels

area-Infrastructure

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@MichalStrehovsky@jkotas@am11@marek-safar@filipnavara@teo-tsirpanis
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Compile native code with -ffunction-sections - #79322
Merged
marek-safar merged 3 commits into
mainfrom 
MichalStrehovsky-patch-1
Dec 7, 2022
Merged
Compile native code with -ffunction-sections#79322
marek-safar merged 3 commits into
mainfrom 
MichalStrehovsky-patch-1
Conversation
@MichalStrehovsky
@MichalStrehovskyMichalStrehovsky
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
We specify /Gy on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
Cc @dotnet/ilc-contrib
@MichalStrehovsky
Compile native code with -ffunction-sections
e0b86fc
We specify `/Gy` on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
@ghost
ghost
commented
Dec 7, 2022

Copy link


Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.
@jkotas
jkotas
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
Need to suppress this for tvOS. clang : error : -ffunction-sections is not supported with -fembed-bitcode
@am11
am11
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
-ffunction-sections is set for debug and checked builds which is now redundant and can be deleted: 
runtime/eng/native/configurecompiler.cmake

Lines 163 to 165
in
e0b86fc
# -fdata-sections -ffunction-sections: each function has own section instead of one per .o file (needed for --gc-sections)
# -O1: optimization level used instead of -O0 to avoid compile error "invalid operand for inline asm constraint"
add_compile_options("$<$<OR:$<CONFIG:DEBUG>,$<CONFIG:CHECKED>>:${CLR_SANITIZE_CXX_OPTIONS};-fdata-sections;--ffunction-sections;-O1>") 
marek-safar
marek-safar
reviewed
Dec 7, 2022
View reviewed changes
Comment threadeng/native/configurecompiler.cmake
Outdated


 # Separate functions so linker can remove them. But not on Apple mobile platforms because
 # -ffunction-sections is not supported with -fembed-bitcode.
 if (NOT CLR_CMAKE_HOST_TVOS AND NOT CLR_CMAKE_HOST_IOS)
@marek-safarmarek-safarDec 7, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should work for iOS, and only tvOS needs be disabled
@MichalStrehovskyMichalStrehovskyDec 7, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saw the failed iOS leg and assumed it's the same problem. But iOS maybe failed for unrelated reasons then. I'll put it back:
.packages/microsoft.dotnet.helix.sdk/8.0.0-beta.22579.2/tools/Microsoft.DotNet.Helix.Sdk.MultiQueue.targets(90,5): error : (NETCORE_ENGINEERING_TELEMETRY=Test) Work item System.Runtime.Tests in job 5143df8d-e525-435f-9a37-d062df01b8cb has failed.

@ghost
ghost
commented
Dec 7, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/runtime-infrastructure

See info in area-owners.md if you want to be subscribed.
Issue Details
We specify /Gy on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
Cc @dotnet/ilc-contrib
Author:MichalStrehovsky
Assignees:MichalStrehovsky
Labels:
area-Infrastructure
Milestone:-
filipnavara
filipnavara
reviewed
Dec 7, 2022
View reviewed changes
Comment threadeng/native/configurecompiler.cmake

Comment on lines

+483
 to 
+487

 # Separate functions so linker can remove them. But not on tvOS because
 # -ffunction-sections is not supported with -fembed-bitcode.
 if (NOT CLR_CMAKE_HOST_TVOS)
 add_compile_options(-ffunction-sections)
 endif()
@filipnavarafilipnavaraDec 7, 2022•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is tvOS still compiled with bitcode? I thought Apple basically deprecated that across the board.
https://developer.apple.com/documentation/Xcode-Release-Notes/xcode-14-release-notes:
Starting with Xcode 14, bitcode is no longer required for watchOS and tvOS applications, and the App Store no longer accepts bitcode submissions from Xcode 14.
Xcode no longer builds bitcode by default and generates a warning message if a project explicitly enables bitcode: “Building with bitcode is deprecated. Please update your project and/or target settings to disable bitcode.” The capability to build with bitcode will be removed in a future Xcode release. IPAs that contain bitcode will have the bitcode stripped before being submitted to the App Store. Debug symbols can only be downloaded from App Store Connect / TestFlight for existing bitcode submissions and are no longer available for submissions made with Xcode 14. (86118779)
@am11am11Dec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is using v13:
Running 'CC="/Applications/Xcode_13.2.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang"..
@filipnavarafilipnavaraDec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it necessarily matters what CI uses. The change is that App Store doesn't require it anymore for new submissions.
@filipnavarafilipnavaraDec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#79349
@marek-safar
marek-safar
merged commit 20af770
into
mainDec 7, 2022
@marek-safar
marek-safar
deleted the

MichalStrehovsky-patch-1

branch
December 7, 2022 17:51
@MichalStrehovskyMichalStrehovsky
mentioned this pull request
Dec 8, 2022
 Merged

@ghostghost
locked as resolved and limited conversation to collaborators
Jan 6, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@marek-safarmarek-safarmarek-safar left review comments
@jkotasjkotasjkotas approved these changes

+2 more reviewers

@filipnavarafilipnavarafilipnavara left review comments
@am11am11am11 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@MichalStrehovskyMichalStrehovsky

Labels

area-Infrastructure

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@MichalStrehovsky@jkotas@am11@marek-safar@filipnavara@teo-tsirpanis

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Compile native code with -ffunction-sections - #79322
Merged
marek-safar merged 3 commits into
mainfrom 
MichalStrehovsky-patch-1
Dec 7, 2022
Merged
Compile native code with -ffunction-sections#79322
marek-safar merged 3 commits into
mainfrom 
MichalStrehovsky-patch-1
Conversation
@MichalStrehovsky
@MichalStrehovskyMichalStrehovsky
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
We specify /Gy on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
Cc @dotnet/ilc-contrib
@MichalStrehovsky
Compile native code with -ffunction-sections
e0b86fc
We specify `/Gy` on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
@ghost
ghost
commented
Dec 7, 2022

Copy link


Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.
@jkotas
jkotas
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
Need to suppress this for tvOS. clang : error : -ffunction-sections is not supported with -fembed-bitcode
@am11
am11
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
-ffunction-sections is set for debug and checked builds which is now redundant and can be deleted: 
runtime/eng/native/configurecompiler.cmake

Lines 163 to 165
in
e0b86fc
# -fdata-sections -ffunction-sections: each function has own section instead of one per .o file (needed for --gc-sections)
# -O1: optimization level used instead of -O0 to avoid compile error "invalid operand for inline asm constraint"
add_compile_options("$<$<OR:$<CONFIG:DEBUG>,$<CONFIG:CHECKED>>:${CLR_SANITIZE_CXX_OPTIONS};-fdata-sections;--ffunction-sections;-O1>") 
marek-safar
marek-safar
reviewed
Dec 7, 2022
View reviewed changes
Comment threadeng/native/configurecompiler.cmake
Outdated


 # Separate functions so linker can remove them. But not on Apple mobile platforms because
 # -ffunction-sections is not supported with -fembed-bitcode.
 if (NOT CLR_CMAKE_HOST_TVOS AND NOT CLR_CMAKE_HOST_IOS)
@marek-safarmarek-safarDec 7, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should work for iOS, and only tvOS needs be disabled
@MichalStrehovskyMichalStrehovskyDec 7, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saw the failed iOS leg and assumed it's the same problem. But iOS maybe failed for unrelated reasons then. I'll put it back:
.packages/microsoft.dotnet.helix.sdk/8.0.0-beta.22579.2/tools/Microsoft.DotNet.Helix.Sdk.MultiQueue.targets(90,5): error : (NETCORE_ENGINEERING_TELEMETRY=Test) Work item System.Runtime.Tests in job 5143df8d-e525-435f-9a37-d062df01b8cb has failed.

@ghost
ghost
commented
Dec 7, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/runtime-infrastructure

See info in area-owners.md if you want to be subscribed.
Issue Details
We specify /Gy on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
Cc @dotnet/ilc-contrib
Author:MichalStrehovsky
Assignees:MichalStrehovsky
Labels:
area-Infrastructure
Milestone:-
filipnavara
filipnavara
reviewed
Dec 7, 2022
View reviewed changes
Comment threadeng/native/configurecompiler.cmake

Comment on lines

+483
 to 
+487

 # Separate functions so linker can remove them. But not on tvOS because
 # -ffunction-sections is not supported with -fembed-bitcode.
 if (NOT CLR_CMAKE_HOST_TVOS)
 add_compile_options(-ffunction-sections)
 endif()
@filipnavarafilipnavaraDec 7, 2022•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is tvOS still compiled with bitcode? I thought Apple basically deprecated that across the board.
https://developer.apple.com/documentation/Xcode-Release-Notes/xcode-14-release-notes:
Starting with Xcode 14, bitcode is no longer required for watchOS and tvOS applications, and the App Store no longer accepts bitcode submissions from Xcode 14.
Xcode no longer builds bitcode by default and generates a warning message if a project explicitly enables bitcode: “Building with bitcode is deprecated. Please update your project and/or target settings to disable bitcode.” The capability to build with bitcode will be removed in a future Xcode release. IPAs that contain bitcode will have the bitcode stripped before being submitted to the App Store. Debug symbols can only be downloaded from App Store Connect / TestFlight for existing bitcode submissions and are no longer available for submissions made with Xcode 14. (86118779)
@am11am11Dec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is using v13:
Running 'CC="/Applications/Xcode_13.2.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang"..
@filipnavarafilipnavaraDec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it necessarily matters what CI uses. The change is that App Store doesn't require it anymore for new submissions.
@filipnavarafilipnavaraDec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#79349
@marek-safar
marek-safar
merged commit 20af770
into
mainDec 7, 2022
@marek-safar
marek-safar
deleted the

MichalStrehovsky-patch-1

branch
December 7, 2022 17:51
@MichalStrehovskyMichalStrehovsky
mentioned this pull request
Dec 8, 2022
 Merged

@ghostghost
locked as resolved and limited conversation to collaborators
Jan 6, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@marek-safarmarek-safarmarek-safar left review comments
@jkotasjkotasjkotas approved these changes

+2 more reviewers

@filipnavarafilipnavarafilipnavara left review comments
@am11am11am11 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@MichalStrehovskyMichalStrehovsky

Labels

area-Infrastructure

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@MichalStrehovsky@jkotas@am11@marek-safar@filipnavara@teo-tsirpanis

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Compile native code with -ffunction-sections - #79322
Merged
marek-safar merged 3 commits into
mainfrom 
MichalStrehovsky-patch-1
Dec 7, 2022
Merged
Compile native code with -ffunction-sections#79322
marek-safar merged 3 commits into
mainfrom 
MichalStrehovsky-patch-1
Conversation
@MichalStrehovsky
@MichalStrehovskyMichalStrehovsky
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
We specify /Gy on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
Cc @dotnet/ilc-contrib
@MichalStrehovsky
Compile native code with -ffunction-sections
e0b86fc
We specify `/Gy` on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
@ghost
ghost
commented
Dec 7, 2022

Copy link


Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.
@jkotas
jkotas
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
Need to suppress this for tvOS. clang : error : -ffunction-sections is not supported with -fembed-bitcode
@am11
am11
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
-ffunction-sections is set for debug and checked builds which is now redundant and can be deleted: 
runtime/eng/native/configurecompiler.cmake

Lines 163 to 165
in
e0b86fc
# -fdata-sections -ffunction-sections: each function has own section instead of one per .o file (needed for --gc-sections)
# -O1: optimization level used instead of -O0 to avoid compile error "invalid operand for inline asm constraint"
add_compile_options("$<$<OR:$<CONFIG:DEBUG>,$<CONFIG:CHECKED>>:${CLR_SANITIZE_CXX_OPTIONS};-fdata-sections;--ffunction-sections;-O1>") 
marek-safar
marek-safar
reviewed
Dec 7, 2022
View reviewed changes
Comment threadeng/native/configurecompiler.cmake
Outdated


 # Separate functions so linker can remove them. But not on Apple mobile platforms because
 # -ffunction-sections is not supported with -fembed-bitcode.
 if (NOT CLR_CMAKE_HOST_TVOS AND NOT CLR_CMAKE_HOST_IOS)
@marek-safarmarek-safarDec 7, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should work for iOS, and only tvOS needs be disabled
@MichalStrehovskyMichalStrehovskyDec 7, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saw the failed iOS leg and assumed it's the same problem. But iOS maybe failed for unrelated reasons then. I'll put it back:
.packages/microsoft.dotnet.helix.sdk/8.0.0-beta.22579.2/tools/Microsoft.DotNet.Helix.Sdk.MultiQueue.targets(90,5): error : (NETCORE_ENGINEERING_TELEMETRY=Test) Work item System.Runtime.Tests in job 5143df8d-e525-435f-9a37-d062df01b8cb has failed.

@ghost
ghost
commented
Dec 7, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/runtime-infrastructure

See info in area-owners.md if you want to be subscribed.
Issue Details
We specify /Gy on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
Cc @dotnet/ilc-contrib
Author:MichalStrehovsky
Assignees:MichalStrehovsky
Labels:
area-Infrastructure
Milestone:-
filipnavara
filipnavara
reviewed
Dec 7, 2022
View reviewed changes
Comment threadeng/native/configurecompiler.cmake

Comment on lines

+483
 to 
+487

 # Separate functions so linker can remove them. But not on tvOS because
 # -ffunction-sections is not supported with -fembed-bitcode.
 if (NOT CLR_CMAKE_HOST_TVOS)
 add_compile_options(-ffunction-sections)
 endif()
@filipnavarafilipnavaraDec 7, 2022•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is tvOS still compiled with bitcode? I thought Apple basically deprecated that across the board.
https://developer.apple.com/documentation/Xcode-Release-Notes/xcode-14-release-notes:
Starting with Xcode 14, bitcode is no longer required for watchOS and tvOS applications, and the App Store no longer accepts bitcode submissions from Xcode 14.
Xcode no longer builds bitcode by default and generates a warning message if a project explicitly enables bitcode: “Building with bitcode is deprecated. Please update your project and/or target settings to disable bitcode.” The capability to build with bitcode will be removed in a future Xcode release. IPAs that contain bitcode will have the bitcode stripped before being submitted to the App Store. Debug symbols can only be downloaded from App Store Connect / TestFlight for existing bitcode submissions and are no longer available for submissions made with Xcode 14. (86118779)
@am11am11Dec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is using v13:
Running 'CC="/Applications/Xcode_13.2.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang"..
@filipnavarafilipnavaraDec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it necessarily matters what CI uses. The change is that App Store doesn't require it anymore for new submissions.
@filipnavarafilipnavaraDec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#79349
@marek-safar
marek-safar
merged commit 20af770
into
mainDec 7, 2022
@marek-safar
marek-safar
deleted the

MichalStrehovsky-patch-1

branch
December 7, 2022 17:51
@MichalStrehovskyMichalStrehovsky
mentioned this pull request
Dec 8, 2022
 Merged

@ghostghost
locked as resolved and limited conversation to collaborators
Jan 6, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@marek-safarmarek-safarmarek-safar left review comments
@jkotasjkotasjkotas approved these changes

+2 more reviewers

@filipnavarafilipnavarafilipnavara left review comments
@am11am11am11 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@MichalStrehovskyMichalStrehovsky

Labels

area-Infrastructure

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@MichalStrehovsky@jkotas@am11@marek-safar@filipnavara@teo-tsirpanis

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Compile native code with -ffunction-sections - #79322
Merged
marek-safar merged 3 commits into
mainfrom 
MichalStrehovsky-patch-1
Dec 7, 2022
Merged
Compile native code with -ffunction-sections#79322
marek-safar merged 3 commits into
mainfrom 
MichalStrehovsky-patch-1
Conversation
@MichalStrehovsky
@MichalStrehovskyMichalStrehovsky
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
We specify /Gy on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
Cc @dotnet/ilc-contrib
@MichalStrehovsky
Compile native code with -ffunction-sections
e0b86fc
We specify `/Gy` on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
@ghost
ghost
commented
Dec 7, 2022

Copy link


Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.
@jkotas
jkotas
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
Need to suppress this for tvOS. clang : error : -ffunction-sections is not supported with -fembed-bitcode
@am11
am11
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
-ffunction-sections is set for debug and checked builds which is now redundant and can be deleted: 
runtime/eng/native/configurecompiler.cmake

Lines 163 to 165
in
e0b86fc
# -fdata-sections -ffunction-sections: each function has own section instead of one per .o file (needed for --gc-sections)
# -O1: optimization level used instead of -O0 to avoid compile error "invalid operand for inline asm constraint"
add_compile_options("$<$<OR:$<CONFIG:DEBUG>,$<CONFIG:CHECKED>>:${CLR_SANITIZE_CXX_OPTIONS};-fdata-sections;--ffunction-sections;-O1>") 
marek-safar
marek-safar
reviewed
Dec 7, 2022
View reviewed changes
Comment threadeng/native/configurecompiler.cmake
Outdated


 # Separate functions so linker can remove them. But not on Apple mobile platforms because
 # -ffunction-sections is not supported with -fembed-bitcode.
 if (NOT CLR_CMAKE_HOST_TVOS AND NOT CLR_CMAKE_HOST_IOS)
@marek-safarmarek-safarDec 7, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should work for iOS, and only tvOS needs be disabled
@MichalStrehovskyMichalStrehovskyDec 7, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saw the failed iOS leg and assumed it's the same problem. But iOS maybe failed for unrelated reasons then. I'll put it back:
.packages/microsoft.dotnet.helix.sdk/8.0.0-beta.22579.2/tools/Microsoft.DotNet.Helix.Sdk.MultiQueue.targets(90,5): error : (NETCORE_ENGINEERING_TELEMETRY=Test) Work item System.Runtime.Tests in job 5143df8d-e525-435f-9a37-d062df01b8cb has failed.

@ghost
ghost
commented
Dec 7, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/runtime-infrastructure

See info in area-owners.md if you want to be subscribed.
Issue Details
We specify /Gy on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
Cc @dotnet/ilc-contrib
Author:MichalStrehovsky
Assignees:MichalStrehovsky
Labels:
area-Infrastructure
Milestone:-
filipnavara
filipnavara
reviewed
Dec 7, 2022
View reviewed changes
Comment threadeng/native/configurecompiler.cmake

Comment on lines

+483
 to 
+487

 # Separate functions so linker can remove them. But not on tvOS because
 # -ffunction-sections is not supported with -fembed-bitcode.
 if (NOT CLR_CMAKE_HOST_TVOS)
 add_compile_options(-ffunction-sections)
 endif()
@filipnavarafilipnavaraDec 7, 2022•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is tvOS still compiled with bitcode? I thought Apple basically deprecated that across the board.
https://developer.apple.com/documentation/Xcode-Release-Notes/xcode-14-release-notes:
Starting with Xcode 14, bitcode is no longer required for watchOS and tvOS applications, and the App Store no longer accepts bitcode submissions from Xcode 14.
Xcode no longer builds bitcode by default and generates a warning message if a project explicitly enables bitcode: “Building with bitcode is deprecated. Please update your project and/or target settings to disable bitcode.” The capability to build with bitcode will be removed in a future Xcode release. IPAs that contain bitcode will have the bitcode stripped before being submitted to the App Store. Debug symbols can only be downloaded from App Store Connect / TestFlight for existing bitcode submissions and are no longer available for submissions made with Xcode 14. (86118779)
@am11am11Dec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is using v13:
Running 'CC="/Applications/Xcode_13.2.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang"..
@filipnavarafilipnavaraDec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it necessarily matters what CI uses. The change is that App Store doesn't require it anymore for new submissions.
@filipnavarafilipnavaraDec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#79349
@marek-safar
marek-safar
merged commit 20af770
into
mainDec 7, 2022
@marek-safar
marek-safar
deleted the

MichalStrehovsky-patch-1

branch
December 7, 2022 17:51
@MichalStrehovskyMichalStrehovsky
mentioned this pull request
Dec 8, 2022
 Merged

@ghostghost
locked as resolved and limited conversation to collaborators
Jan 6, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@marek-safarmarek-safarmarek-safar left review comments
@jkotasjkotasjkotas approved these changes

+2 more reviewers

@filipnavarafilipnavarafilipnavara left review comments
@am11am11am11 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@MichalStrehovskyMichalStrehovsky

Labels

area-Infrastructure

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@MichalStrehovsky@jkotas@am11@marek-safar@filipnavara@teo-tsirpanis

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Compile native code with -ffunction-sections - #79322
Merged
marek-safar merged 3 commits into
mainfrom 
MichalStrehovsky-patch-1
Dec 7, 2022
Merged
Compile native code with -ffunction-sections#79322
marek-safar merged 3 commits into
mainfrom 
MichalStrehovsky-patch-1
Conversation
@MichalStrehovsky
@MichalStrehovskyMichalStrehovsky
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
We specify /Gy on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
Cc @dotnet/ilc-contrib
@MichalStrehovsky
Compile native code with -ffunction-sections
e0b86fc
We specify `/Gy` on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
@ghost
ghost
commented
Dec 7, 2022

Copy link


Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.
@jkotas
jkotas
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
Need to suppress this for tvOS. clang : error : -ffunction-sections is not supported with -fembed-bitcode
@am11
am11
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
-ffunction-sections is set for debug and checked builds which is now redundant and can be deleted: 
runtime/eng/native/configurecompiler.cmake

Lines 163 to 165
in
e0b86fc
# -fdata-sections -ffunction-sections: each function has own section instead of one per .o file (needed for --gc-sections)
# -O1: optimization level used instead of -O0 to avoid compile error "invalid operand for inline asm constraint"
add_compile_options("$<$<OR:$<CONFIG:DEBUG>,$<CONFIG:CHECKED>>:${CLR_SANITIZE_CXX_OPTIONS};-fdata-sections;--ffunction-sections;-O1>") 
marek-safar
marek-safar
reviewed
Dec 7, 2022
View reviewed changes
Comment threadeng/native/configurecompiler.cmake
Outdated


 # Separate functions so linker can remove them. But not on Apple mobile platforms because
 # -ffunction-sections is not supported with -fembed-bitcode.
 if (NOT CLR_CMAKE_HOST_TVOS AND NOT CLR_CMAKE_HOST_IOS)
@marek-safarmarek-safarDec 7, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should work for iOS, and only tvOS needs be disabled
@MichalStrehovskyMichalStrehovskyDec 7, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saw the failed iOS leg and assumed it's the same problem. But iOS maybe failed for unrelated reasons then. I'll put it back:
.packages/microsoft.dotnet.helix.sdk/8.0.0-beta.22579.2/tools/Microsoft.DotNet.Helix.Sdk.MultiQueue.targets(90,5): error : (NETCORE_ENGINEERING_TELEMETRY=Test) Work item System.Runtime.Tests in job 5143df8d-e525-435f-9a37-d062df01b8cb has failed.

@ghost
ghost
commented
Dec 7, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/runtime-infrastructure

See info in area-owners.md if you want to be subscribed.
Issue Details
We specify /Gy on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
Cc @dotnet/ilc-contrib
Author:MichalStrehovsky
Assignees:MichalStrehovsky
Labels:
area-Infrastructure
Milestone:-
filipnavara
filipnavara
reviewed
Dec 7, 2022
View reviewed changes
Comment threadeng/native/configurecompiler.cmake

Comment on lines

+483
 to 
+487

 # Separate functions so linker can remove them. But not on tvOS because
 # -ffunction-sections is not supported with -fembed-bitcode.
 if (NOT CLR_CMAKE_HOST_TVOS)
 add_compile_options(-ffunction-sections)
 endif()
@filipnavarafilipnavaraDec 7, 2022•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is tvOS still compiled with bitcode? I thought Apple basically deprecated that across the board.
https://developer.apple.com/documentation/Xcode-Release-Notes/xcode-14-release-notes:
Starting with Xcode 14, bitcode is no longer required for watchOS and tvOS applications, and the App Store no longer accepts bitcode submissions from Xcode 14.
Xcode no longer builds bitcode by default and generates a warning message if a project explicitly enables bitcode: “Building with bitcode is deprecated. Please update your project and/or target settings to disable bitcode.” The capability to build with bitcode will be removed in a future Xcode release. IPAs that contain bitcode will have the bitcode stripped before being submitted to the App Store. Debug symbols can only be downloaded from App Store Connect / TestFlight for existing bitcode submissions and are no longer available for submissions made with Xcode 14. (86118779)
@am11am11Dec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is using v13:
Running 'CC="/Applications/Xcode_13.2.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang"..
@filipnavarafilipnavaraDec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it necessarily matters what CI uses. The change is that App Store doesn't require it anymore for new submissions.
@filipnavarafilipnavaraDec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#79349
@marek-safar
marek-safar
merged commit 20af770
into
mainDec 7, 2022
@marek-safar
marek-safar
deleted the

MichalStrehovsky-patch-1

branch
December 7, 2022 17:51
@MichalStrehovskyMichalStrehovsky
mentioned this pull request
Dec 8, 2022
 Merged

@ghostghost
locked as resolved and limited conversation to collaborators
Jan 6, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@marek-safarmarek-safarmarek-safar left review comments
@jkotasjkotasjkotas approved these changes

+2 more reviewers

@filipnavarafilipnavarafilipnavara left review comments
@am11am11am11 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@MichalStrehovskyMichalStrehovsky

Labels

area-Infrastructure

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@MichalStrehovsky@jkotas@am11@marek-safar@filipnavara@teo-tsirpanis

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Compile native code with -ffunction-sections - #79322
Merged
marek-safar merged 3 commits into
mainfrom 
MichalStrehovsky-patch-1
Dec 7, 2022
Merged
Compile native code with -ffunction-sections#79322
marek-safar merged 3 commits into
mainfrom 
MichalStrehovsky-patch-1
Conversation
@MichalStrehovsky
@MichalStrehovskyMichalStrehovsky
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
We specify /Gy on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
Cc @dotnet/ilc-contrib
@MichalStrehovsky
Compile native code with -ffunction-sections
e0b86fc
We specify `/Gy` on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
@ghost
ghost
commented
Dec 7, 2022

Copy link


Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.
@jkotas
jkotas
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
Need to suppress this for tvOS. clang : error : -ffunction-sections is not supported with -fembed-bitcode
@am11
am11
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
-ffunction-sections is set for debug and checked builds which is now redundant and can be deleted: 
runtime/eng/native/configurecompiler.cmake

Lines 163 to 165
in
e0b86fc
# -fdata-sections -ffunction-sections: each function has own section instead of one per .o file (needed for --gc-sections)
# -O1: optimization level used instead of -O0 to avoid compile error "invalid operand for inline asm constraint"
add_compile_options("$<$<OR:$<CONFIG:DEBUG>,$<CONFIG:CHECKED>>:${CLR_SANITIZE_CXX_OPTIONS};-fdata-sections;--ffunction-sections;-O1>") 
marek-safar
marek-safar
reviewed
Dec 7, 2022
View reviewed changes
Comment threadeng/native/configurecompiler.cmake
Outdated


 # Separate functions so linker can remove them. But not on Apple mobile platforms because
 # -ffunction-sections is not supported with -fembed-bitcode.
 if (NOT CLR_CMAKE_HOST_TVOS AND NOT CLR_CMAKE_HOST_IOS)
@marek-safarmarek-safarDec 7, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should work for iOS, and only tvOS needs be disabled
@MichalStrehovskyMichalStrehovskyDec 7, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saw the failed iOS leg and assumed it's the same problem. But iOS maybe failed for unrelated reasons then. I'll put it back:
.packages/microsoft.dotnet.helix.sdk/8.0.0-beta.22579.2/tools/Microsoft.DotNet.Helix.Sdk.MultiQueue.targets(90,5): error : (NETCORE_ENGINEERING_TELEMETRY=Test) Work item System.Runtime.Tests in job 5143df8d-e525-435f-9a37-d062df01b8cb has failed.

@ghost
ghost
commented
Dec 7, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/runtime-infrastructure

See info in area-owners.md if you want to be subscribed.
Issue Details
We specify /Gy on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
Cc @dotnet/ilc-contrib
Author:MichalStrehovsky
Assignees:MichalStrehovsky
Labels:
area-Infrastructure
Milestone:-
filipnavara
filipnavara
reviewed
Dec 7, 2022
View reviewed changes
Comment threadeng/native/configurecompiler.cmake

Comment on lines

+483
 to 
+487

 # Separate functions so linker can remove them. But not on tvOS because
 # -ffunction-sections is not supported with -fembed-bitcode.
 if (NOT CLR_CMAKE_HOST_TVOS)
 add_compile_options(-ffunction-sections)
 endif()
@filipnavarafilipnavaraDec 7, 2022•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is tvOS still compiled with bitcode? I thought Apple basically deprecated that across the board.
https://developer.apple.com/documentation/Xcode-Release-Notes/xcode-14-release-notes:
Starting with Xcode 14, bitcode is no longer required for watchOS and tvOS applications, and the App Store no longer accepts bitcode submissions from Xcode 14.
Xcode no longer builds bitcode by default and generates a warning message if a project explicitly enables bitcode: “Building with bitcode is deprecated. Please update your project and/or target settings to disable bitcode.” The capability to build with bitcode will be removed in a future Xcode release. IPAs that contain bitcode will have the bitcode stripped before being submitted to the App Store. Debug symbols can only be downloaded from App Store Connect / TestFlight for existing bitcode submissions and are no longer available for submissions made with Xcode 14. (86118779)
@am11am11Dec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is using v13:
Running 'CC="/Applications/Xcode_13.2.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang"..
@filipnavarafilipnavaraDec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it necessarily matters what CI uses. The change is that App Store doesn't require it anymore for new submissions.
@filipnavarafilipnavaraDec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#79349
@marek-safar
marek-safar
merged commit 20af770
into
mainDec 7, 2022
@marek-safar
marek-safar
deleted the

MichalStrehovsky-patch-1

branch
December 7, 2022 17:51
@MichalStrehovskyMichalStrehovsky
mentioned this pull request
Dec 8, 2022
 Merged

@ghostghost
locked as resolved and limited conversation to collaborators
Jan 6, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@marek-safarmarek-safarmarek-safar left review comments
@jkotasjkotasjkotas approved these changes

+2 more reviewers

@filipnavarafilipnavarafilipnavara left review comments
@am11am11am11 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@MichalStrehovskyMichalStrehovsky

Labels

area-Infrastructure

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@MichalStrehovsky@jkotas@am11@marek-safar@filipnavara@teo-tsirpanis

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Compile native code with -ffunction-sections - #79322
Merged
marek-safar merged 3 commits into
mainfrom 
MichalStrehovsky-patch-1
Dec 7, 2022
Merged
Compile native code with -ffunction-sections#79322
marek-safar merged 3 commits into
mainfrom 
MichalStrehovsky-patch-1
Conversation
@MichalStrehovsky
@MichalStrehovskyMichalStrehovsky
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
We specify /Gy on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
Cc @dotnet/ilc-contrib
@MichalStrehovsky
Compile native code with -ffunction-sections
e0b86fc
We specify `/Gy` on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
@ghost
ghost
commented
Dec 7, 2022

Copy link


Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.
@jkotas
jkotas
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
Need to suppress this for tvOS. clang : error : -ffunction-sections is not supported with -fembed-bitcode
@am11
am11
commented
Dec 7, 2022

Copy link


Copy Markdown

Member
-ffunction-sections is set for debug and checked builds which is now redundant and can be deleted: 
runtime/eng/native/configurecompiler.cmake

Lines 163 to 165
in
e0b86fc
# -fdata-sections -ffunction-sections: each function has own section instead of one per .o file (needed for --gc-sections)
# -O1: optimization level used instead of -O0 to avoid compile error "invalid operand for inline asm constraint"
add_compile_options("$<$<OR:$<CONFIG:DEBUG>,$<CONFIG:CHECKED>>:${CLR_SANITIZE_CXX_OPTIONS};-fdata-sections;--ffunction-sections;-O1>") 
marek-safar
marek-safar
reviewed
Dec 7, 2022
View reviewed changes
Comment threadeng/native/configurecompiler.cmake
Outdated


 # Separate functions so linker can remove them. But not on Apple mobile platforms because
 # -ffunction-sections is not supported with -fembed-bitcode.
 if (NOT CLR_CMAKE_HOST_TVOS AND NOT CLR_CMAKE_HOST_IOS)
@marek-safarmarek-safarDec 7, 2022

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should work for iOS, and only tvOS needs be disabled
@MichalStrehovskyMichalStrehovskyDec 7, 2022

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saw the failed iOS leg and assumed it's the same problem. But iOS maybe failed for unrelated reasons then. I'll put it back:
.packages/microsoft.dotnet.helix.sdk/8.0.0-beta.22579.2/tools/Microsoft.DotNet.Helix.Sdk.MultiQueue.targets(90,5): error : (NETCORE_ENGINEERING_TELEMETRY=Test) Work item System.Runtime.Tests in job 5143df8d-e525-435f-9a37-d062df01b8cb has failed.

@ghost
ghost
commented
Dec 7, 2022

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/runtime-infrastructure

See info in area-owners.md if you want to be subscribed.
Issue Details
We specify /Gy on Windows, but don't do this on non-Windows. This looks like an omission. For NativeAOT, doing this saves ~140 kB in size on a Hello World.
Cc @dotnet/ilc-contrib
Author:MichalStrehovsky
Assignees:MichalStrehovsky
Labels:
area-Infrastructure
Milestone:-
filipnavara
filipnavara
reviewed
Dec 7, 2022
View reviewed changes
Comment threadeng/native/configurecompiler.cmake

Comment on lines

+483
 to 
+487

 # Separate functions so linker can remove them. But not on tvOS because
 # -ffunction-sections is not supported with -fembed-bitcode.
 if (NOT CLR_CMAKE_HOST_TVOS)
 add_compile_options(-ffunction-sections)
 endif()
@filipnavarafilipnavaraDec 7, 2022•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is tvOS still compiled with bitcode? I thought Apple basically deprecated that across the board.
https://developer.apple.com/documentation/Xcode-Release-Notes/xcode-14-release-notes:
Starting with Xcode 14, bitcode is no longer required for watchOS and tvOS applications, and the App Store no longer accepts bitcode submissions from Xcode 14.
Xcode no longer builds bitcode by default and generates a warning message if a project explicitly enables bitcode: “Building with bitcode is deprecated. Please update your project and/or target settings to disable bitcode.” The capability to build with bitcode will be removed in a future Xcode release. IPAs that contain bitcode will have the bitcode stripped before being submitted to the App Store. Debug symbols can only be downloaded from App Store Connect / TestFlight for existing bitcode submissions and are no longer available for submissions made with Xcode 14. (86118779)
@am11am11Dec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is using v13:
Running 'CC="/Applications/Xcode_13.2.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang"..
@filipnavarafilipnavaraDec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it necessarily matters what CI uses. The change is that App Store doesn't require it anymore for new submissions.
@filipnavarafilipnavaraDec 7, 2022

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#79349
@marek-safar
marek-safar
merged commit 20af770
into
mainDec 7, 2022
@marek-safar
marek-safar
deleted the

MichalStrehovsky-patch-1

branch
December 7, 2022 17:51
@MichalStrehovskyMichalStrehovsky
mentioned this pull request
Dec 8, 2022
 Merged

@ghostghost
locked as resolved and limited conversation to collaborators
Jan 6, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@marek-safarmarek-safarmarek-safar left review comments
@jkotasjkotasjkotas approved these changes

+2 more reviewers

@filipnavarafilipnavarafilipnavara left review comments
@am11am11am11 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@MichalStrehovskyMichalStrehovsky

Labels

area-Infrastructure

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@MichalStrehovsky@jkotas@am11@marek-safar@filipnavara@teo-tsirpanis