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

[wasm] Fix regression in compiling .bc -> .o files #56063

Merged
merged 8 commits into from
Jul 21, 2021

Conversation

radical
Copy link
Member

@radical radical commented Jul 21, 2021

The -emit-llvm arg has been incorrectly added, and removed from the
args used for compiling .bc->.o couple of times. Added a crude test to
avoid regressing in the future.

  • And reduces EmccCompile's output to Low importance, so it doesn't pollute a normal build output (eg. for Blazor)
  • And add some more tests for blazorwasm

@ghost
Copy link

ghost commented Jul 21, 2021

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

.. including Add back --emit-llvm that got removed mistakenly, in an earlier commit.

Author: radical
Assignees: -
Labels:

arch-wasm, area-Build-mono

Milestone: -

@radical radical requested a review from radekdoulik July 21, 2021 00:25
@@ -181,6 +183,7 @@
<_EmccCFlags Include="-DLINK_ICALLS=1" Condition="'$(WasmLinkIcalls)' == 'true'" />
<_EmccCFlags Include="-DCORE_BINDINGS" />
<_EmccCFlags Include="-DGEN_PINVOKE=1" />
<_EmccCFlags Include="-emit-llvm" />

Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this be passed to the .bc -> .o builds as well ?

Copy link
Member Author

Choose a reason for hiding this comment

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

So, looks like I broke this in the last few days! Adding here is fine, because it will get used only to compile the .c files. And the bitcode files will use the LDFlags.

I've added a crude test to check for this, so we don't regress again.

The `-emit-llvm` arg has been incorrectly added, and removed from the
args used for compiling .bc->.o .

This commit fixes it, and adds a crude test for it, so we don't regress
again.
@radical radical requested a review from vargaz July 21, 2021 13:03
@radical radical changed the title [wasm] Few misc build improvements [wasm] Fix regression in compiling .bc -> .o files Jul 21, 2021
@karelz
Copy link
Member

karelz commented Jul 21, 2021

Http3_MsQuic test failures are tracked in #56090. Should be fixed and merged shortly. Sorry for the inconvenience!

@lewing lewing merged commit 1d8ad03 into dotnet:main Jul 21, 2021
radical added a commit to radical/runtime that referenced this pull request Jul 22, 2021
* [wasm] Add back --emit-llvm that got removed mistakenly, in an earlier commit

.. found thanks to Jerome Laban.

* [wasm] Set EmccCompile's messages to MessageImportance.Low by default.

.. and to MessageImportance.Normal if `$(EmccVerbose)==true`.

* [wasm] Quote filenames passed to emcc compile command line

* Add more blazorwasm tests - for debug/release, aot/relinking

* Bump sdk for workload testing to 6.0.100-rc.1.21370.2

* [wasm] Fix regression in compiling bitcode -> .o

The `-emit-llvm` arg has been incorrectly added, and removed from the
args used for compiling .bc->.o .

This commit fixes it, and adds a crude test for it, so we don't regress
again.

* Fix build

(cherry picked from commit 1d8ad03)
mmitche pushed a commit that referenced this pull request Jul 26, 2021
…raries (#56013)

* [wasm] Add support for using custom native libraries (#55797)

(cherry picked from commit d574b03)

* [wasm] Use compile rsp instead of link, for compiling native files (#55848)

.. and fix logging that broke recently.

`tasks/Common/Utils.cs`:

TaskLoggingHelper Utils.Logger is a static field, which must be set by
task else any methods in Utils, eg. RunProcess, silently fail to log
any messages. Also, this would be a problem when building multiple
projects in parallel, since the logger is a task-specific one.

Instead, we pass logger as an arg to all the methods.

(cherry picked from commit 3301e9d)

* Link with EmccCompileOptimizationFlag==-Oz by default in release (#55939)

(cherry picked from commit 04072ff)

* [wasm] Fix regression in compiling .bc -> .o files (#56063)

* [wasm] Add back --emit-llvm that got removed mistakenly, in an earlier commit

.. found thanks to Jerome Laban.

* [wasm] Set EmccCompile's messages to MessageImportance.Low by default.

.. and to MessageImportance.Normal if `$(EmccVerbose)==true`.

* [wasm] Quote filenames passed to emcc compile command line

* Add more blazorwasm tests - for debug/release, aot/relinking

* Bump sdk for workload testing to 6.0.100-rc.1.21370.2

* [wasm] Fix regression in compiling bitcode -> .o

The `-emit-llvm` arg has been incorrectly added, and removed from the
args used for compiling .bc->.o .

This commit fixes it, and adds a crude test for it, so we don't regress
again.

* Fix build

(cherry picked from commit 1d8ad03)

* [wasm] Bump sdk for workload testing to 6.0.100-preview.7.21372.19

Co-authored-by: Larry Ewing <lewing@microsoft.com>
@ghost ghost locked as resolved and limited conversation to collaborators Aug 20, 2021
@radical radical deleted the wasm-misc-improvements branch August 20, 2021 19:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Build-mono
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants