erichkeane added a comment.
So one thing to note here: I'm on the fence as to whether we want to implement
this feature at all. As was discussed extensively during the EWG meetings on
this: multiple implementers are against this attribute for a variety of
reasons, and at least 1 other implementer has stated they might 'implementer
veto' this. I think there is discussion to be had among the code owners here as
to whether we even want this.
================
Comment at: clang/include/clang/Basic/AttrDocs.td:2107
+ let Content = [{
+The ``assume`` attribute used to provide the optimizer with an expression that
+is defined to evaluate to ``true`` at the place the assumption occurs. The
----------------
================
Comment at: clang/include/clang/Basic/AttrDocs.td:2112
+violated during execution, the behavior is undefined. The argument itself is
+never evaluated, so any side effects of the expression will be discarded.
+
----------------
This last sentence is inaccurate. As discussed in EWG, this attribute has some
pretty painful side-effects (instantiation, lambda creation/etc).
================
Comment at: clang/test/CodeGenCXX/cxx2b-assume.cpp:2
+// RUN: %clang_cc1 -std=c++2b %s -emit-llvm -o - -triple x86_64-linux-gnu
-disable-llvm-passes -DUSE_ASSUME | FileCheck %s
+// RUN: %clang_cc1 -std=c++2b %s -emit-llvm -o - -triple x86_64-linux-gnu -O3
| FileCheck --check-prefixes=CHECK-OPT-NOASSUME %s
+// RUN: %clang_cc1 -std=c++2b %s -emit-llvm -o - -triple x86_64-linux-gnu -O3
-DUSE_ASSUME | FileCheck --check-prefixes=CHECK-OPT %s
----------------
Having a CFE test depend on the optimizer like this is EXTREMELY frowned upon.
We shouldn't have clang tests depend on optimizer performance.
================
Comment at: clang/test/CodeGenCXX/cxx2b-assume.cpp:55
+// CHECK-OPT-NEXT: tail call void @llvm.assume(i1 %[[CMP]])
+// CHECK-OPT-NEXT: ret i32 43
+//
----------------
nikic wrote:
> Isn't the assume expression supposed to be unevaluated?
We need MUCH more complicated tests (including just any expression/function
call/etc) to ensure that the side-effects aren't going to cause problems,
particularly in -O0. I am somewhat confident that the middle-end is going to
be willing to do the work to figure out that something passed to 'assume' is
passed out correctly, but I'm not convinced that this will prevent all
side-effects from escaping.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144334/new/
https://reviews.llvm.org/D144334
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
