-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[Clang] Enable -fextend-variable-liveness at -Og #118026
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
Conversation
|
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Stephen Tozer (SLTozer) ChangesThis patch follows two other ongoing reviews, #110000 and #110102, which add a new feature to Clang - Following the inclusion of the flag, this patch enables it by default when Full diff: https://github.com/llvm/llvm-project/pull/118026.diff 4 Files Affected:
diff --git a/clang/docs/CommandGuide/clang.rst b/clang/docs/CommandGuide/clang.rst
index ca8176f854729b..8f3f0de9446b41 100644
--- a/clang/docs/CommandGuide/clang.rst
+++ b/clang/docs/CommandGuide/clang.rst
@@ -442,8 +442,11 @@ Code Generation Options
:option:`-Oz` Like :option:`-Os` (and thus :option:`-O2`), but reduces code
size further.
- :option:`-Og` Like :option:`-O1`. In future versions, this option might
- disable different optimizations in order to improve debuggability.
+ :option:`-Og` Similar to :option:`-O1`, but with slightly reduced
+ optimization and better variable visibility. The same optimizations are run
+ as at :option:`-O1`, but the :option:`-fextend-lifetimes` flag is also
+ set, which tries to prevent optimizations from reducing the lifetime of
+ user variables.
:option:`-O` Equivalent to :option:`-O1`.
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 601a233b81904f..14fec70dfb85f7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -441,6 +441,10 @@ Modified Compiler Flags
``memset`` and similar functions for which it is a documented undefined
behavior. It is implied by ``-Wnontrivial-memaccess``
+- The ``-Og`` optimization flag now sets ``-fextend-lifetimes``, a new compiler
+ flag, resulting in slightly reduced optimization compared to ``-O1`` in
+ exchange for improved variable visibility.
+
Removed Compiler Flags
-------------------------
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 3dd94c31b2bc7a..09322d0617aaad 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1834,6 +1834,14 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
Opts.setInlining(CodeGenOptions::NormalInlining);
}
+ // If we have specified -Og and have not set any explicit -fextend-lifetimes
+ // value, then default to -fextend-lifetimes=all.
+ if (Arg *A = Args.getLastArg(options::OPT_O_Group);
+ A && A->containsValue("g")) {
+ if (!Args.hasArg(options::OPT_fextend_lifetimes))
+ Opts.ExtendLifetimes = true;
+ }
+
// PIC defaults to -fno-direct-access-external-data while non-PIC defaults to
// -fdirect-access-external-data.
Opts.DirectAccessExternalData =
diff --git a/clang/test/CodeGen/fake-use-opt-flags.cpp b/clang/test/CodeGen/fake-use-opt-flags.cpp
new file mode 100644
index 00000000000000..a99ed7a573ab22
--- /dev/null
+++ b/clang/test/CodeGen/fake-use-opt-flags.cpp
@@ -0,0 +1,30 @@
+/// Check that we generate fake uses only when -fextend-lifetimes is set and we
+/// are not setting optnone, or when we have optimizations set to -Og and we have
+/// not passed -fno-extend-lifetimes.
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O0 -fextend-lifetimes %s -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O0 -disable-O0-optnone -fextend-lifetimes %s -o - | FileCheck %s --check-prefixes=CHECK,EXTEND
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -Og %s -o - | FileCheck %s --check-prefixes=CHECK,EXTEND
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -Og -fno-extend-lifetimes %s -o - | FileCheck %s
+
+/// Test various optimization flags with -fextend-lifetimes...
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O1 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O1 -fextend-lifetimes %s -o - | FileCheck %s --check-prefixes=CHECK,EXTEND
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O2 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O2 -fextend-lifetimes %s -o - | FileCheck %s --check-prefixes=CHECK,EXTEND
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O3 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O3 -fextend-lifetimes %s -o - | FileCheck %s --check-prefixes=CHECK,EXTEND
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -Os %s -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -Os -fextend-lifetimes %s -o - | FileCheck %s --check-prefixes=CHECK,EXTEND
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -Oz %s -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -Oz -fextend-lifetimes %s -o - | FileCheck %s --check-prefixes=CHECK,EXTEND
+
+// CHECK-LABEL: define{{.*}} void @_Z3fooi(i32{{.*}} %a)
+// CHECK-NEXT: entry:
+// CHECK-NEXT: %a.addr = alloca i32
+// CHECK-NEXT: store i32 %a, ptr %a.addr
+// EXTEND-NEXT: %fake.use = load i32, ptr %a.addr
+// EXTEND-NEXT: call void (...) @llvm.fake.use(i32 %fake.use)
+// CHECK-NEXT: ret void
+// CHECK-NEXT: }
+
+void foo(int a) {}
|
|
@llvm/pr-subscribers-clang-codegen Author: Stephen Tozer (SLTozer) ChangesThis patch follows two other ongoing reviews, #110000 and #110102, which add a new feature to Clang - Following the inclusion of the flag, this patch enables it by default when Full diff: https://github.com/llvm/llvm-project/pull/118026.diff 4 Files Affected:
diff --git a/clang/docs/CommandGuide/clang.rst b/clang/docs/CommandGuide/clang.rst
index ca8176f854729b..8f3f0de9446b41 100644
--- a/clang/docs/CommandGuide/clang.rst
+++ b/clang/docs/CommandGuide/clang.rst
@@ -442,8 +442,11 @@ Code Generation Options
:option:`-Oz` Like :option:`-Os` (and thus :option:`-O2`), but reduces code
size further.
- :option:`-Og` Like :option:`-O1`. In future versions, this option might
- disable different optimizations in order to improve debuggability.
+ :option:`-Og` Similar to :option:`-O1`, but with slightly reduced
+ optimization and better variable visibility. The same optimizations are run
+ as at :option:`-O1`, but the :option:`-fextend-lifetimes` flag is also
+ set, which tries to prevent optimizations from reducing the lifetime of
+ user variables.
:option:`-O` Equivalent to :option:`-O1`.
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 601a233b81904f..14fec70dfb85f7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -441,6 +441,10 @@ Modified Compiler Flags
``memset`` and similar functions for which it is a documented undefined
behavior. It is implied by ``-Wnontrivial-memaccess``
+- The ``-Og`` optimization flag now sets ``-fextend-lifetimes``, a new compiler
+ flag, resulting in slightly reduced optimization compared to ``-O1`` in
+ exchange for improved variable visibility.
+
Removed Compiler Flags
-------------------------
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 3dd94c31b2bc7a..09322d0617aaad 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1834,6 +1834,14 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
Opts.setInlining(CodeGenOptions::NormalInlining);
}
+ // If we have specified -Og and have not set any explicit -fextend-lifetimes
+ // value, then default to -fextend-lifetimes=all.
+ if (Arg *A = Args.getLastArg(options::OPT_O_Group);
+ A && A->containsValue("g")) {
+ if (!Args.hasArg(options::OPT_fextend_lifetimes))
+ Opts.ExtendLifetimes = true;
+ }
+
// PIC defaults to -fno-direct-access-external-data while non-PIC defaults to
// -fdirect-access-external-data.
Opts.DirectAccessExternalData =
diff --git a/clang/test/CodeGen/fake-use-opt-flags.cpp b/clang/test/CodeGen/fake-use-opt-flags.cpp
new file mode 100644
index 00000000000000..a99ed7a573ab22
--- /dev/null
+++ b/clang/test/CodeGen/fake-use-opt-flags.cpp
@@ -0,0 +1,30 @@
+/// Check that we generate fake uses only when -fextend-lifetimes is set and we
+/// are not setting optnone, or when we have optimizations set to -Og and we have
+/// not passed -fno-extend-lifetimes.
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O0 -fextend-lifetimes %s -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O0 -disable-O0-optnone -fextend-lifetimes %s -o - | FileCheck %s --check-prefixes=CHECK,EXTEND
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -Og %s -o - | FileCheck %s --check-prefixes=CHECK,EXTEND
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -Og -fno-extend-lifetimes %s -o - | FileCheck %s
+
+/// Test various optimization flags with -fextend-lifetimes...
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O1 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O1 -fextend-lifetimes %s -o - | FileCheck %s --check-prefixes=CHECK,EXTEND
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O2 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O2 -fextend-lifetimes %s -o - | FileCheck %s --check-prefixes=CHECK,EXTEND
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O3 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O3 -fextend-lifetimes %s -o - | FileCheck %s --check-prefixes=CHECK,EXTEND
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -Os %s -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -Os -fextend-lifetimes %s -o - | FileCheck %s --check-prefixes=CHECK,EXTEND
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -Oz %s -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -Oz -fextend-lifetimes %s -o - | FileCheck %s --check-prefixes=CHECK,EXTEND
+
+// CHECK-LABEL: define{{.*}} void @_Z3fooi(i32{{.*}} %a)
+// CHECK-NEXT: entry:
+// CHECK-NEXT: %a.addr = alloca i32
+// CHECK-NEXT: store i32 %a, ptr %a.addr
+// EXTEND-NEXT: %fake.use = load i32, ptr %a.addr
+// EXTEND-NEXT: call void (...) @llvm.fake.use(i32 %fake.use)
+// CHECK-NEXT: ret void
+// CHECK-NEXT: }
+
+void foo(int a) {}
|
jmorse
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor test comments, otherwise looking good,
| } | ||
|
|
||
| // If we have specified -Og and have not set any explicit -fextend-lifetimes | ||
| // value, then default to -fextend-lifetimes=all. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is going to create confusion by speculating the existence of an ={all,params,this} fextend-lifetimes when it doesn't exist yet, and might not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I implemented the = option on a local-only branch, and accidentally left this comment in. My mistake, though I may yet decide to merge the feature into the earlier PRs.
clang/docs/ReleaseNotes.rst
Outdated
| behavior. It is implied by ``-Wnontrivial-memaccess`` | ||
|
|
||
| - The ``-Og`` optimization flag now sets ``-fextend-lifetimes``, a new compiler | ||
| flag, resulting in slightly reduced optimization compared to ``-O1`` in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just say "trading optimization for improved..." to indicate that this is a conscious decision the developer might make?
| /// Check that we generate fake uses only when -fextend-lifetimes is set and we | ||
| /// are not setting optnone, or when we have optimizations set to -Og and we have | ||
| /// not passed -fno-extend-lifetimes. | ||
| // RUN: %clang_cc1 -emit-llvm -disable-llvm-passes -O0 -fextend-lifetimes %s -o - | FileCheck %s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intention of this RUN line is that no fake uses are generated, but there are no test-lines for ensuring that -> add an implicit-fake-not? And the other check-only FileCheck commands.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally I would use implicit-check-not, but in this case we're CHECK-NEXTing from the start of the function to the closing brace, which equally guarantees that no fake uses have been generated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We just need two RUN lines, one with -fextended-lifetimes and one without. -O group is completely orthogonal and does not need to be tested
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A->getValue() == "g"
Perhaps move the code around ToolChains/Clang.cpp:9198 and let the driver pass -fextend-lifetimes to frontend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an advantage to having it passed through the driver? I ask mainly because I see -fextend-lifetimes as being a feature of -Og optimization, and if you invoke clang -cc1 -Og directly then you'd expect/hope that the optimization behaviour would be the same as clang -Og - I'm not too familiar with expected Clang behaviour though, so if this is quite normal for driver/frontend behaviour then I'm fine to switch it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if we have a fundamental principle about how flags should work in the driver V the frontend, but yeah - in some ways the driver acts as a lowering/canonicalizer - representing flag defaults, etc. So I think the expectation that clang -cc1 -Og does the same thing as clang -Og isn't valid/not something we're striving for (& in some sense counter to what we are striving for: that all the defaults/grouping is handled by the driver, and the frontend can just think about whether a flag is enabled or disabled)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, the driver's job is usually to unpack high level flags into semi-orthogonal cc1 flags. So, for example, in clang-cl, /O1 becomes a collection of things like -Os -ffunction-sections -fdata-sections and maybe something else I've forgotten. clang++ -fexceptions becomes -fexceptions -fcxx-exceptions in most contexts, separating destructor cleanups from try/throw language support.
cc1 flags kind of represent features that are useful to toggle on and off for testing and test case reduction, since they're one of the things we reduce in the clang crash reducer script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've moved this behaviour to the driver now. Also regarding the first request to use A->getValue() == "g" - if just -O is passed then A->getValue() will crash, as there are no values. An alternative would be to change the check to A->getNumValues() && A->getValue() == "g", but I think containsValue is more concise.
ec32824 to
2b59ce7
Compare
|
I've updated this patch, and made it dependent on a previous PR - the logic for enabling the flag is coupled with code from the earlier PR, and also we now slightly extend the test added in that PR instead of creating a new one. To see just the code added in this patch, ignore the first commit. |
jmorse
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, but I'd like to reword some of it
clang/docs/ReleaseNotes.rst
Outdated
| Clang to generate code that tries to preserve the liveness of source variables | ||
| through optimizations, meaning that variables will typically be visible in a | ||
| debugger more often. The flag has two levels: ``-fextend-variable-liveness``, | ||
| or ``-fextend-variable-liveness=all``, extendes the liveness of all user |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| or ``-fextend-variable-liveness=all``, extendes the liveness of all user | |
| or ``-fextend-variable-liveness=all``, extends the liveness of all user |
clang/docs/ReleaseNotes.rst
Outdated
| in reduced performance in generated code; however, this feature will not | ||
| extend the liveness of some variables in cases where doing so would likely | ||
| have a severe impact on generated code performance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, just drop the late sentence for brevity -- it's up to the user to work out whether the trade-off is worth it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intention here is just to specify to the user that some variables won't have their liveness extended, so that they're not caught off-guard if they notice some variables getting excluded.
| HelpText<"Extend the liveness of user variables through optimizations to " | ||
| "prevent stale or optimized-out variable values when debugging. Can " | ||
| "be applied to all user variables, or just to the C++ 'this' ptr. " | ||
| "May choose not to extend the liveness of some variables, such as " | ||
| "non-scalars larger than 4 unsigned ints, or variables in any " | ||
| "inlined functions.">, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there other compiler flags which have such long help-text? IIRC this is a super-short summary that appears on the command line with --help, and thus conciseness is key. Developers looking for more information will go to the full docs.
2b59ce7 to
beb5e19
Compare
|
With the prior patches having landed, this is now in a state where it could land; I've rebased it onto main, meaning all of the code diff in this PR is now relevant. |
rnk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this ready to go? I can't recall if we ultimately accepted the RFC.
My interpretation of the RFC is that there was a general acceptance of |
beb5e19 to
e503747
Compare
e503747 to
7126cb5
Compare
|
We started seeing the following failure, and I bisected it to this commit. I'm uploading a reproducer. Any ideas why we might be hitting this issue? This happens during a built with |
At a glance that looks like it could be related, we generate (no-op) cleanups as part of -fextend-variable-liveness. I'll look into this, but could you clarify what you mean - did you mean that the error occurs with |
Fixes the issue reported following the merge of llvm#118026. When a valid `musttail` call is made, the function it is made from must return immediately after the call; if there are any cleanups left in the function, then an error is triggered. This is not necessary for fake uses however - it is perfectly valid to simply emit the fake use "cleanup" code before the tail call, and indeed LLVM will automatically move any fake uses following a tail call to come before the tail call. Therefore, this patch specifically choose to handle fake use cleanups when a musttail call is present by simply emitting them immediately before the call.
I've created a fix for this issue here: #136867 |
Yes, it does not reproduce if I remove |
|
I did some downstream testing with "-Og" since this patch and noticed that e.g. crashes with and fails with for bbi-106478.ll just being (and if removing the llvm.fake.use call they don't crash) |
Fixes the issue reported following the merge of #118026. When a valid `musttail` call is made, the function it is made from must return immediately after the call; if there are any cleanups left in the function, then an error is triggered. This is not necessary for fake uses however - it is perfectly valid to simply emit the fake use "cleanup" code before the tail call, and indeed LLVM will automatically move any fake uses following a tail call to come before the tail call. Therefore, this patch specifically choose to handle fake use cleanups when a musttail call is present by simply emitting them immediately before the call.
Thanks for catching these - in terms of legalizer support it looks like everything relevant is implemented except for I'll revert this for now (after verifying), since it may take a little time to confirm that changing |
Thanks! |
Reverted following several issues appearing related to fake uses, reported on the github PR. This reverts commit a9dff35.
Recently, a new flag -fextend-variable-liveness was added that prevents optimizations from removing the values of source variables in some cases, improving the quality of debugging for optimized builds where it is enabled. Following the inclusion of the flag, this patch enables it by default when `-Og` is set. Currently, `-Og` is equivalent to `-O1` - it is effectively just an alias. By enabling `-fextend-lifetimes`, this patch changes the code generated by Clang with `-Og` to have reduced optimization and greater debuggability than `-O1`, differentiating the two according to their respective purposes. This idea was discussed previously on Discourse where there was general agreement with the principle of this change: https://discourse.llvm.org/t/rfc-redefine-og-o1-and-add-a-new-level-of-og
Fixes the issue reported following the merge of llvm#118026. When a valid `musttail` call is made, the function it is made from must return immediately after the call; if there are any cleanups left in the function, then an error is triggered. This is not necessary for fake uses however - it is perfectly valid to simply emit the fake use "cleanup" code before the tail call, and indeed LLVM will automatically move any fake uses following a tail call to come before the tail call. Therefore, this patch specifically choose to handle fake use cleanups when a musttail call is present by simply emitting them immediately before the call.
Reverted following several issues appearing related to fake uses, reported on the github PR. This reverts commit a9dff35.
|
Since the option has been renamed to Also, make sure that you update https://discourse.llvm.org/t/rfc-redefine-og-o1-and-add-a-new-level-of-og/72850/25 after the option relands after the musttail fix. |
Relands this feature after several fixes: * Force fake uses to be emitted before musttail calls (#136867) * Added soften-float legalization for fake uses (#142714) * Treat fake uses as size-less instructions in a SystemZ assert (#144390) If further issues with fake uses are found then this may be reverted again, but all currently-known issues are resolved. This reverts commit 2dc6e98.
|
This change seems to have broken some emscripten users downstream: emscripten-core/emscripten#25301 Seems likely a bug in WebAssembly backend, but posting here in case anything looks obvious/strange. |
|
There's a corresponding intrinsic llvm.fake.use (FAKE_USE in MIR), which isn't used by anything else. Possibly there's a bug in the implementation of that intrinsic. |
I'm not familiar with emscripten/wasm/binaryen; do you have the last state of the IR/MIR? Potentially the I don't know exactly how to read the binaryen IR, but for some speculation: the "fake use" feature that was added by this change intentionally preserves values that are not used by the program but that are (likely to be) referenced by debug information, to prevent variables from appearing optimized out. At a glance, the error in question seems to be that binaryen expects some values in the block to have been dropped, which could be related to the existence of fake use intrinsics forcing an unused value to be emitted? |
This patch follows two other ongoing reviews, #110000 and #110102, which add a new feature to Clang -
-fextend-variable-liveness, which generates fake uses for user variables (and thethispointer) to prevent them from being optimized out, providing a better debug experience at the cost of less efficient generated code. This is useful for debugging code without the need to disable optimizations outright, which is particularly important when debugging applications where some level of optimization is necessary, e.g. game code.Following the inclusion of the flag, this patch enables it by default when
-Ogis set. Currently,-Ogis equivalent to-O1- it is effectively just an alias. By enabling-fextend-variable-liveness, this patch changes the code generated by Clang with-Ogto have reduced optimization and greater debuggability than-O1, differentiating the two according to their respective purposes. This idea was discussed previously on Discourse, where there was general agreement with the principle of this change.