jmgao added a comment.
With #1, it seems unfortunate to not be able to distinguish between a sanitize
inserted __builtin_trap and code manually calling it. (Would there be an
-fsanitize-trap=trap? :-)
With #2, we're worried about the generated code being noticeably worse in the
unexceptional ca
rsmith added a comment.
In http://reviews.llvm.org/D12181#234590, @jmgao wrote:
> Ping, I think @samsonov was waiting on @rsmith's feedback on the following:
>
> In http://reviews.llvm.org/D12181#229493, @jmgao wrote:
>
> > The goal is to be able to give a useful fsanitize-specific error message
jmgao added a comment.
Ping, I think @samsonov was waiting on @rsmith's feedback on the following:
In http://reviews.llvm.org/D12181#229493, @jmgao wrote:
> In http://reviews.llvm.org/D12181#229467, @rsmith wrote:
>
> > In http://reviews.llvm.org/D12181#229358, @rsmith wrote:
> >
> > > Can you p
jmgao marked 4 inline comments as done.
jmgao added a comment.
http://reviews.llvm.org/D12181
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jmgao marked an inline comment as done.
Comment at: lib/CodeGen/CGExpr.cpp:2303
@@ -2302,4 +2302,3 @@
- if (TrapCond)
-EmitTrapCheck(TrapCond);
+ if (TrapCond) EmitSanitizeTrapCheck(TrapCond);
if (!FatalCond && !RecoverableCond)
Yes
http://reviews.llv
jmgao updated this revision to Diff 32833.
jmgao added a comment.
Remove more `\brief`s
http://reviews.llvm.org/D12181
Files:
docs/UsersManual.rst
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.h
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CodeGenFunction.h
lib/Fronten
filcab added a comment.
It seems you missed some `\brief`. Other than that, no complaints on this side.
Comment at: lib/CodeGen/CGExpr.cpp:2303
@@ -2302,4 +2302,3 @@
- if (TrapCond)
-EmitTrapCheck(TrapCond);
+ if (TrapCond) EmitSanitizeTrapCheck(TrapCond);
if (!FatalC
jmgao updated this revision to Diff 32788.
jmgao marked 7 inline comments as done.
jmgao added a comment.
clang-format, remove \brief from modified doxygen comments.
http://reviews.llvm.org/D12181
Files:
docs/UsersManual.rst
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOp
filcab added a subscriber: filcab.
filcab added a comment.
Looks good, thanks for working on this.
I added some code style comments.
Comment at: lib/CodeGen/CGExpr.cpp:2399
@@ -2385,1 +2398,3 @@
+
+void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked, const std::string
&Tra
jmgao updated this revision to Diff 32776.
jmgao marked 2 inline comments as done.
jmgao added a comment.
Make methods private
http://reviews.llvm.org/D12181
Files:
docs/UsersManual.rst
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.h
lib/CodeGen/CGExpr.cpp
lib/
jmgao updated this revision to Diff 32769.
jmgao added a comment.
Doc fix, 80 col
http://reviews.llvm.org/D12181
Files:
docs/UsersManual.rst
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.h
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CodeGenFunction.h
lib/Frontend/Comp
jmgao updated this revision to Diff 32768.
jmgao added a comment.
Make option fit in 80 cols
http://reviews.llvm.org/D12181
Files:
docs/UsersManual.rst
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.h
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CodeGenFunction.h
lib/Fr
jmgao added a comment.
In http://reviews.llvm.org/D12181#229467, @rsmith wrote:
> In http://reviews.llvm.org/D12181#229358, @rsmith wrote:
>
> > Can you please give a brief description of the motivation for this change?
> > When would it be appropriate to use this rather than `-ftrap-function`?
rsmith added a comment.
In http://reviews.llvm.org/D12181#229358, @rsmith wrote:
> Can you please give a brief description of the motivation for this change?
> When would it be appropriate to use this rather than `-ftrap-function`?
I'd still like an answer to this. It's not clear to me what th
samsonov added a comment.
Overall, this looks reasonable to me, but I'd like Richard to confirm he's fine
with this change as well.
Comment at: docs/UsersManual.rst:1121
@@ +1120,3 @@
+ Instruct code generator to emit a function call to the specified
+ function name instead
jmgao updated this revision to Diff 32756.
jmgao added a comment.
Improve comment
http://reviews.llvm.org/D12181
Files:
docs/UsersManual.rst
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.h
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CodeGenFunction.h
lib/Frontend/Comp
jmgao updated this revision to Diff 32755.
jmgao marked 2 inline comments as done.
jmgao added a comment.
Address comments
http://reviews.llvm.org/D12181
Files:
docs/UsersManual.rst
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.h
lib/CodeGen/CGExpr.cpp
lib/Code
jmgao marked 8 inline comments as done.
Comment at: tools/clang/lib/CodeGen/CGExpr.cpp:2388
@@ +2387,3 @@
+ }
+ return EmitTrapCheck(Checked);
+}
samsonov wrote:
> This is confusing. So, you have the following behavior whenever you need to
> emit a check for `-
samsonov added a comment.
See https://llvm.org/bugs/show_bug.cgi?id=24443 (it's worth including this
reference to change description).
http://reviews.llvm.org/D12181
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-
rsmith added a comment.
Can you please give a brief description of the motivation for this change? When
would it be appropriate to use this rather than `-ftrap-function`?
Please also include an update for the Clang documentation to describe the new
flag.
http://reviews.llvm.org/D12181
jmgao updated this revision to Diff 32736.
jmgao added a comment.
Uploading diff with arcanist.
http://reviews.llvm.org/D12181
Files:
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.h
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGExprScalar.cpp
lib/CodeGen/CodeGenFunction
samsonov added a comment.
Please upload the patch with more context (see
http://llvm.org/docs/Phabricator.html).
Comment at: tools/clang/include/clang/Driver/Options.td:610
@@ -608,1 +609,3 @@
+ Flags<[CC1Option, CoreOption]>,
+
22 matches
Mail list logo
