1668373 - Remove CompilerEnvironment from ModuleEnvironment

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, task, P3)

Description

[Ryan Hunt [:rhunt]]

This is purely a code cleanup task.

ModuleEnvironment contains a CompilerEnvironment pointer, which describes the compiler selection, tiering, and whether debugging is enabled. None of this information is needed for validation, so wasm::Validate() has to create a synthetic CompilerEnvironment which is never read. This isn't a large issue, but indicates ModuleEnvironment is doing too many things.

I think it would be cleaner if we passed the CompilerEnvironment along with the ModuleEnvironment through the compiler pipeline, and removed this responsibility from ModuleEnvironment.

Comment 1

[Ryan Hunt [:rhunt]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7b8865166e4a648846f60ecc7d04dd9c8ccf3669

Comment 2

[Ryan Hunt [:rhunt]]

Attachment: Bug 1668373 - wasm: Rename future ambiguous uses of 'env_' to 'moduleEnv_'. r?lth

The next commit will move CompilerEnvironment out of ModuleEnvironment and pass it
through the compiler pipeline. The compiler pipeline typically uses 'env_' to refer
to the module environment, with a 'compilerEnv_' being used as well I think we should
rename 'env_' to 'moduleEnv_'.

The one exception is within validation/decoding where I think using just 'env_' to
refer to the module environment is fine.

Comment 3

[Ryan Hunt [:rhunt]]

Attachment: Bug 1668373 - wasm: Split CompilerEnvironment from ModuleEnvironment. r?lth

ModuleEnvironment contains a CompilerEnvironment pointer, which describes the compiler selection,
tiering, and whether debugging is enabled. None of this information is needed for validation,
so wasm::Validate() has to create a synthetic CompilerEnvironment which is never read. This
isn't a large issue, but indicates ModuleEnvironment is doing too many things.

This commit removes CompilerEnvironment from ModuleEnvironment and pipes it
through the compiler pipeline. This is fairly straightforward except for:

• wasm::Validate() no longer needs to create a synthetic compiler env
• DecodeModuleEnvironment() used to invoke moduleEnv.compilerEnv.computeParameters(d)
  after the preamble was decoded. I believe this was needed for handling the GC opt-in
  section, which is no longer used. I moved this call to computeParameters() to after
  all callers of DecodeModuleEnvironment().
• I added an assertion in IonCompileFunctions/CraneliftCompileFunctions that they are
  not being invoked for debugged modules.

Depends on D91994

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9652ae0962ac
wasm: Rename future ambiguous uses of 'env_' to 'moduleEnv_'. r=lth
https://hg.mozilla.org/integration/autoland/rev/cd62c4bc2412
wasm: Split CompilerEnvironment from ModuleEnvironment. r=lth

Comment 5

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/9652ae0962ac
https://hg.mozilla.org/mozilla-central/rev/cd62c4bc2412