Re: [PATCH] D23498: Left shifts of negative values are defined if -fwrapv is set

2016-08-16 Thread James Molloy via cfe-commits
jmolloy accepted this revision. jmolloy added a reviewer: jmolloy. jmolloy added a comment. This revision is now accepted and ready to land. Thanks, r278786! https://reviews.llvm.org/D23498 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D23498: Left shifts of negative values are defined if -fwrapv is set

2016-08-16 Thread Filipe Cabecinhas via cfe-commits
LGTM. Thank you, Filipe On Tue, Aug 16, 2016 at 10:23 AM, Davide Italiano wrote: > davide added a comment. > > The `Sema` bits look fine to me. I'll let Filipe comment on the sanitizer > part. > > > https://reviews.llvm.org/D23498 > > >

Re: [PATCH] D23498: Left shifts of negative values are defined if -fwrapv is set

2016-08-16 Thread Davide Italiano via cfe-commits
davide added a comment. The `Sema` bits look fine to me. I'll let Filipe comment on the sanitizer part. https://reviews.llvm.org/D23498 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D23498: Left shifts of negative values are defined if -fwrapv is set

2016-08-16 Thread James Molloy via cfe-commits
jmolloy added a comment. Hi Filipe, Did this look good to you after those changes? Cheers, James https://reviews.llvm.org/D23498 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23498: Left shifts of negative values are defined if -fwrapv is set

2016-08-15 Thread James Molloy via cfe-commits
jmolloy updated this revision to Diff 68033. jmolloy added a comment. Hi Filipe, Thanks for the review! SGTM on both counts. https://reviews.llvm.org/D23498 Files: lib/CodeGen/CGExprScalar.cpp lib/Sema/SemaExpr.cpp test/CodeGen/wrapv-lshr-sanitize.c test/Sema/negative-shift-wrapv.c

Re: [PATCH] D23498: Left shifts of negative values are defined if -fwrapv is set

2016-08-15 Thread Filipe Cabecinhas via cfe-commits
filcab added a subscriber: filcab. Comment at: test/CodeGen/wrapv-lshr-sanitize.c:1 @@ +1,2 @@ +// RUN: %clang_cc1 -fsanitize=shift-base -emit-llvm %s -o - -triple x86_64-linux-gnu -fwrapv | opt -instnamer -S | FileCheck %s + Do you really need `instnamer`?

[PATCH] D23498: Left shifts of negative values are defined if -fwrapv is set

2016-08-15 Thread James Molloy via cfe-commits
jmolloy created this revision. jmolloy added reviewers: davide, aaron.ballman. jmolloy added a subscriber: cfe-commits. This means we shouldn't emit ubsan detection code or warn. Fixes PR25552. https://reviews.llvm.org/D23498 Files: lib/CodeGen/CGExprScalar.cpp lib/Sema/SemaExpr.cpp