[PATCH] D74094: Reapply: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

[David Tellenbach via Phabricator via cfe-commits]

tellenbach added a comment.

No real comment on the issue itself but on the example as a former Eigen 
maintainer (sorry for the noise if that's all obvious for you):

  auto round (Tensor m) {
  return (m + 0.5f).cast().cast();
  }

does not return a Tensor but an expression encoding the computation which is 
evaluated during the assignment `const Tensor t3 = round(a.log().exp());` using 
an overloaded `operator=`. With "evaluation" I mean evaluation the in the sense 
of performing the intended mathematical computation.

Many things can go wrong when using `auto` in such cases, of which two seem to 
be relevant here:

1. Eigen can (this is an implementation detail(!)) decide to evaluate 
subexpressions into temporaries. The returned expression would then reference 
the result of such an evaluation beyond its lifetime.
2. If 1. does not happen, the unevaluated expression would reference its 
arguments. The immediate `0.5f` is copied since that's cheap, but the tensor 
would be referenced. Your example function `round` takes its parameter by-value 
and the returned expression would reference it. I'm unsure if the lifetime 
would be extended in this case (again, the exact details of C++ lifetime rules 
are not my area of expertise). I think if the lifetime would be extended, ASAN 
complaining after applying this patch is wrong, if the lifetime is not extended 
ASAN should complain and the example is wrong.

Btw, these issues are so common that Eigen documents the recommendation to 
avoid using `auto` with Eigen types all together: 
https://eigen.tuxfamily.org/dox/TopicPitfalls.html#title3


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74094/new/

https://reviews.llvm.org/D74094

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157925: [MachineOutliner][LTO] Enable outlining of linkonceodr functions on all targets.

[David Tellenbach via Phabricator via cfe-commits]

tellenbach accepted this revision.
tellenbach added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157925/new/

https://reviews.llvm.org/D157925

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68076: [AArch64] Enable unwind tables by default for Gnu targets

[David Tellenbach via Phabricator via cfe-commits]

tellenbach added a comment.

In D68076#2131108 , @hiraditya wrote:

> do we have a plan to follow up on this patch?


Hi! I'm currently not working on this (and LLVM in general) anymore. However, 
the issue seems to be still present and this patch would fix it. I have in mind 
that I've checked GCC recently and they seem to do something similar. I can 
check again but will probably take some time.

Thanks for bringing this up again!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68076/new/

https://reviews.llvm.org/D68076



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68076: [AArch64] Enable unwind tables by default for Gnu targets

[David Tellenbach via Phabricator via cfe-commits]

tellenbach added a comment.

This patch would fix  PR37240 but is probably not the most optimal solution. 
For other ideas see 
http://lists.llvm.org/pipermail/llvm-dev/2019-September/135433.html 
. I'll 
leave this patch open until other possibilities have been accepted or abandoned 
and will abandon or fix this patch then.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68076/new/

https://reviews.llvm.org/D68076



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68076: [AArch64] Enable unwind tables by default for Gnu targets

[David Tellenbach via Phabricator via cfe-commits]

tellenbach added a comment.

This would fix PR37240 


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68076/new/

https://reviews.llvm.org/D68076



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68076: [AArch64] Enable unwind tables by default for Gnu targets

[David Tellenbach via Phabricator via cfe-commits]

tellenbach added a comment.

Please also see the ongoing discussion on the mailing list: 
http://lists.llvm.org/pipermail/llvm-dev/2019-September/135433.html 



Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68076/new/

https://reviews.llvm.org/D68076



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68076: [AArch64] Enable unwind tables by default for Gnu targets

[David Tellenbach via Phabricator via cfe-commits]

tellenbach created this revision.
Herald added subscribers: cfe-commits, kristof.beyls, aprantl.
Herald added a project: clang.

Currently the following situation can occur: During AArch64 frame lowering cfi 
instructions get emitted if debug information is generated but not if not. 
Since cfi instructions act as scheduling boundaries during instruction 
scheduling this leads to different scheduling regions and therefore different 
assembly depending on whether debug information is generated or not.

This patch fixes this problem by enabling unwind tables by default on AArch64 
when compiling for a Gnu targets.


Repository:
  rC Clang

https://reviews.llvm.org/D68076

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/clang-translation.c


Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -150,6 +150,10 @@
 // ARMv8_THREAD_POINTER_EL3-NOT: "-target-feature" "+tpidr-el2"
 // ARMv8_THREAD_POINTER_EL3: "-target-feature" "+tpidr-el3"
 
+// RUN: %clang -target aarch64-linux -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=AARCH64_LINUX_DEFAULT_UNWIND_TABLES %s
+// AARCH64_LINUX_DEFAULT_UNWIND_TABLES: "-munwind-tables"
+
 // RUN: %clang -target powerpc64-unknown-linux-gnu \
 // RUN: -### -S %s -mcpu=G5 2>&1 | FileCheck -check-prefix=PPCG5 %s
 // PPCG5: clang
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2540,7 +2540,8 @@
 }
 
 bool Generic_GCC::IsUnwindTablesDefault(const ArgList ) const {
-  return getArch() == llvm::Triple::x86_64;
+  return getArch() == llvm::Triple::x86_64 ||
+ getArch() == llvm::Triple::aarch64;
 }
 
 bool Generic_GCC::isPICDefault() const {


Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -150,6 +150,10 @@
 // ARMv8_THREAD_POINTER_EL3-NOT: "-target-feature" "+tpidr-el2"
 // ARMv8_THREAD_POINTER_EL3: "-target-feature" "+tpidr-el3"
 
+// RUN: %clang -target aarch64-linux -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=AARCH64_LINUX_DEFAULT_UNWIND_TABLES %s
+// AARCH64_LINUX_DEFAULT_UNWIND_TABLES: "-munwind-tables"
+
 // RUN: %clang -target powerpc64-unknown-linux-gnu \
 // RUN: -### -S %s -mcpu=G5 2>&1 | FileCheck -check-prefix=PPCG5 %s
 // PPCG5: clang
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2540,7 +2540,8 @@
 }
 
 bool Generic_GCC::IsUnwindTablesDefault(const ArgList ) const {
-  return getArch() == llvm::Triple::x86_64;
+  return getArch() == llvm::Triple::x86_64 ||
+ getArch() == llvm::Triple::aarch64;
 }
 
 bool Generic_GCC::isPICDefault() const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits