[Clang] Enable -fextend-variable-liveness at -Og

[SLTozer]

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 the this pointer) 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 -Og is set. Currently, -Og is equivalent to -O1 - it is effectively just an alias. By enabling -fextend-variable-liveness, 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.

[llvmbot]

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Stephen Tozer (SLTozer)

Changes

This patch follows two other ongoing reviews, #110000 and #110102, which add a new feature to Clang - -fextend-lifetimes, which generates fake uses for user variables (and the this pointer) 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 -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.

---

Full diff: https://github.com/llvm/llvm-project/pull/118026.diff

4 Files Affected:

• (modified) clang/docs/CommandGuide/clang.rst (+5-2)
• (modified) clang/docs/ReleaseNotes.rst (+4)
• (modified) clang/lib/Frontend/CompilerInvocation.cpp (+8)
• (added) clang/test/CodeGen/fake-use-opt-flags.cpp (+30)

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) {}

[llvmbot]

@llvm/pr-subscribers-clang-codegen

Author: Stephen Tozer (SLTozer)

Changes

This patch follows two other ongoing reviews, #110000 and #110102, which add a new feature to Clang - -fextend-lifetimes, which generates fake uses for user variables (and the this pointer) 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 -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.

---

Full diff: https://github.com/llvm/llvm-project/pull/118026.diff

4 Files Affected:

• (modified) clang/docs/CommandGuide/clang.rst (+5-2)
• (modified) clang/docs/ReleaseNotes.rst (+4)
• (modified) clang/lib/Frontend/CompilerInvocation.cpp (+8)
• (added) clang/test/CodeGen/fake-use-opt-flags.cpp (+30)

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]

Some minor test comments, otherwise looking good,

[jmorse]

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.

[SLTozer]

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.

[jmorse]

Can we just say "trading optimization for improved..." to indicate that this is a conscious decision the developer might make?

[jmorse]

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.

[SLTozer]

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.

[MaskRay]

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

[MaskRay]

A->getValue() == "g"

Perhaps move the code around ToolChains/Clang.cpp:9198 and let the driver pass -fextend-lifetimes to frontend.

[SLTozer]

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.

[dwblaikie]

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)

[rnk]

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.

[SLTozer]

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.

[SLTozer]

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]

This looks good, but I'd like to reword some of it

[jmorse]

Suggested change

	or ``-fextend-variable-liveness=all``, extendes the liveness of all user
	or ``-fextend-variable-liveness=all``, extends the liveness of all user

[jmorse]

IMO, just drop the late sentence for brevity -- it's up to the user to work out whether the trade-off is worth it.

[SLTozer]

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.

[jmorse]

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.

[SLTozer]

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]

Is this ready to go? I can't recall if we ultimately accepted the RFC.

[SLTozer]

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 -Og = -O1 + -fextend-variable-liveness, so with the acceptance here I believe this is ready!

[gulfemsavrun]

We started seeing the following failure, and I bisected it to this commit.

[1343/43547](319) CXX host_x64/obj/third_party/protobuf/src/google/protobuf/libprotobuf_lite.generated_message_tctable_lite.cc.o
FAILED: [code=1] host_x64/obj/third_party/protobuf/src/google/protobuf/libprotobuf_lite.generated_message_tctable_lite.cc.o 
../../build/rbe/reclient_cxx.sh --working-subdir=out/not-default --exec_strategy=remote_local_fallback --preserve_unchanged_output_mtime --  ../../prebuilt/third_party/clang/linux-x64/bin/clang++ -MD -MF host_x64/obj/third_party/protobuf/src/google/protobuf/libprotobuf_lite.generated_message_tctable_lite.cc.o.d -DTOOLCHAIN_VERSION=xBAWf3R2sN1ViZaJjFVJTCaaAZEuAtpSDTaGs7iC35IC -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES -DGOOGLE_PROTOBUF_NO_RTTI -DHAVE_PTHREAD -I../.. -Ihost_x64/gen -I../../third_party/protobuf/src -I../../third_party/protobuf/third_party/utf8_range -I../../third_party/abseil-cpp -fcolor-diagnostics -fcrash-diagnostics-dir=clang-crashreports -fcrash-diagnostics=all -ffp-contract=off --sysroot=../../prebuilt/third_party/sysroot/linux --target=x86_64-unknown-linux-gnu -ffile-compilation-dir=. -no-canonical-prefixes -fomit-frame-pointer -fdata-sections -ffunction-sections -Og -Xclang -debug-info-kind=constructor -g3 -grecord-gcc-switches -gdwarf-5 -gz=zstd -Wall -Wextra -Wconversion -Wextra-semi -Wimplicit-fallthrough -Wnewline-eof -Wstrict-prototypes -Wwrite-strings -Wno-sign-conversion -Wno-unused-parameter -Wnonportable-system-include-path -Wno-missing-field-initializers -Wno-extra-qualification -Wno-cast-function-type-strict -Wno-cast-function-type-mismatch -Wno-unknown-warning-option -Wno-missing-template-arg-list-after-template-kw -Wno-deprecated-pragma -Wno-nontrivial-memaccess -fvisibility=hidden -Werror -Wa,--fatal-warnings --sysroot=../../prebuilt/third_party/sysroot/linux --target=x86_64-unknown-linux-gnu -Wno-deprecated-pragma -Wno-enum-enum-conversion -Wno-extra-semi -Wno-float-conversion -Wno-implicit-float-conversion -Wno-implicit-int-conversion -Wno-implicit-int-float-conversion -Wno-invalid-noreturn -Wno-missing-field-initializers -Wno-sign-compare -Wno-unused-function -Wno-unused-private-field -Wno-deprecated-declarations -Wno-deprecated-this-capture -Wno-unnecessary-virtual-specifier -Wno-c++98-compat-extra-semi -Wno-shorten-64-to-32 -Wno-sign-compare -Wno-missing-field-initializers -Wno-conversion -Wno-unknown-warning-option -Wno-implicit-int-float-conversion -Wno-array-parameter -Wno-deprecated-builtins -fvisibility-inlines-hidden -stdlib=libc++ -stdlib=libc++ -std=c++20 -Wno-deprecated-this-capture -fno-exceptions -fno-rtti -ftemplate-backtrace-limit=0 -stdlib=libc++ -c ../../third_party/protobuf/src/google/protobuf/generated_message_tctable_lite.cc -o host_x64/obj/third_party/protobuf/src/google/protobuf/libprotobuf_lite.generated_message_tctable_lite.cc.o
../../third_party/protobuf/src/google/protobuf/generated_message_tctable_lite.cc:68:28: error: cannot compile this tail call skipping over cleanups yet
   68 |   PROTOBUF_MUSTTAIL return GenericFallbackImpl<MessageLite, std::string>(
      |                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   69 |       PROTOBUF_TC_PARAM_PASS);
      |       ~~~~~~~~~~~~~~~~~~~~~~~

I'm uploading a reproducer.
clang-crashreports.zip

Any ideas why we might be hitting this issue? This happens during a built with -Og, and issue does not remove if I remove -Og flag.

[SLTozer]

Any ideas why we might be hitting this issue? This happens during a built with -Og, and issue does not remove if I remove -Og flag.

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 -Og and does not occur when you remove -Og?

[SLTozer]

We started seeing the following failure, and I bisected it to this commit.

I've created a fix for this issue here: #136867

[gulfemsavrun]

Any ideas why we might be hitting this issue? This happens during a built with -Og, and issue does not remove if I remove -Og flag.

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 -Og and does not occur when you remove -Og?

Yes, it does not reproduce if I remove -Og.

[mikaelholmen]

I did some downstream testing with "-Og" since this patch and noticed that e.g.

llc -mtriple=avr bbi-106478.ll

crashes with

SoftenFloatOperand Op #1: t2: ch = fake_use t0, ConstantFP:f64<0.000000e+00>

LLVM ERROR: Do not know how to soften this operator's operand!

and

llc -mtriple=systemz bbi-106478.ll

fails with

llc: ../lib/Target/SystemZ/SystemZLongBranch.cpp:226: unsigned int getInstSizeInBytes(const MachineInstr &, const SystemZInstrInfo *): Assertion `(Size || MI.isDebugOrPseudoInstr() || MI.isPosition() || MI.isKill() || MI.isImplicitDef() || MI.getOpcode() == TargetOpcode::MEMBARRIER || MI.getOpcode() == TargetOpcode::INIT_UNDEF || MI.isInlineAsm() || MI.getOpcode() == SystemZ::STACKMAP || MI.getOpcode() == SystemZ::PATCHPOINT || MI.getOpcode() == SystemZ::EH_SjLj_Setup) && "Missing size value for instruction."' failed.

for bbi-106478.ll just being

define double @idd() {
entry:
  notail call void (...) @llvm.fake.use(double 0.000000e+00)
  ret double 0.000000e+00
}

(and if removing the llvm.fake.use call they don't crash)
so I guess there is some testing and fixes around llvm.fake.use left to do.

[SLTozer]

I did some downstream testing with "-Og" since this patch and noticed that e.g.

Thanks for catching these - in terms of legalizer support it looks like everything relevant is implemented except for SoftenFloatOperand, so that should be a straightforward fix. The target-specific error is also trivial (FAKE_USE has a size of 0), but it would be preferable if every target didn't need to account for it - previously there was a discussion of whether FAKE_USE should have isPosition, and this probably settles that since it would allow them to be treated correctly by-default.

I'll revert this for now (after verifying), since it may take a little time to confirm that changing isPosition() doesn't introduce any new bugs; I'll try to expand the fake use tests to cover more targets at the same time.

[mikaelholmen]

I'll revert this for now (after verifying), since it may take a little time to confirm that changing isPosition() doesn't introduce any new bugs; I'll try to expand the fake use tests to cover more targets at the same time.

Thanks!

[MaskRay]

Since the option has been renamed to -fextend-variable-liveness, I think you want to edit this title and the description to reflect the naming change as well. I wanted to check the status the effort and I tried git log --grep fextend-lifetimes but found nothing, then I realized that the option has been renamed :)

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.