[PATCH] D30415: Fix -mno-altivec cannot overwrite -maltivec option

2017-03-14 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D30415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

[PATCH] D30415: Fix -mno-altivec cannot overwrite -maltivec option

2017-03-14 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D30415#700534, @nemanjai wrote: > This seems to change the relationship between -m[no-]altivec and > -f[no-]altivec which has been kind of contentious for a while. Can you add a > note as to whether the new behaviour matches the GCC behaviour

[PATCH] D30920: Do not pass -Os and -Oz to the Gold plugin

2017-03-14 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D30920#700557, @mehdi_amini wrote: > In https://reviews.llvm.org/D30920#700433, @tejohnson wrote: > > > In https://reviews.llvm.org/D30920#700133, @pcc wrote: > > > > > In https://reviews.llvm.org/D30920#700077, @tejohnson wrote: > > > > > > >

[PATCH] D30923: Warn on enum assignment to bitfields that can't fit all values

2017-03-14 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D30923#700620, @thakis wrote: > Maybe it should have some "to suppress the warning, do X" fixit? I think we should at least include how many bits the field should have to fix the problem (pointing to the relevant field definition certainly s

[PATCH] D23765: Fix for clang PR 29087

2016-11-27 Thread Hal Finkel via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL287999: Adjust type-trait evaluation to properly handle Using(Shadow)Decls (authored by hfinkel). Changed prior to commit: https://reviews.llvm.org/D23765?vs=76817&id=79354#toc Repository: rL LLVM h

[PATCH] D27180: Testbed and skeleton of a new expression parser

2016-11-28 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. This seems like a great idea. btw, do you know about Cling (https://root.cern.ch/cling)? Repository: rL LLVM https://reviews.llvm.org/D27180 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D27304: [PATCH] [Sema][X86] Don't allow floating-point return types when SSE is disabled

2016-12-01 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D27304#610944, @thegameg wrote: > In https://reviews.llvm.org/D27304#610697, @joerg wrote: > > > I think this is the absolutely wrong place to put such logic. It really can > > not be anywhere but the backend. > > > I am aware of this. But the

[PATCH] D25686: [Driver] Support "hardfloat" vendor triples used by Gentoo

2016-12-01 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D25686#571894, @mgorny wrote: > In https://reviews.llvm.org/D25686#571857, @hfinkel wrote: > > > It seems like we should teach Triple how to parse this triples correctly > > (i.e. to recognize that the vendor is ARM), and then canonicalize the

[PATCH] D27351: [CUDA] Forward sanitizer support to host toolchain

2016-12-02 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel accepted this revision. hfinkel added a reviewer: hfinkel. hfinkel added a comment. LGTM https://reviews.llvm.org/D27351 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25435: Add -femit-accurate-debug-info to emit more debug info for sample pgo profile collection

2016-12-11 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D25435#609320, @danielcdh wrote: > change the flag name to -fprofile-debug I don't really like this name because it sounds like it might be enabling some kind of debugging of the profiling. How about -fdebug-info-for-profiling. Another opti

[PATCH] D24933: Enable configuration files in clang

2016-12-12 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added inline comments. Comment at: lib/Driver/Driver.cpp:2695 + // Claim all arguments that come from configuration file, - driver must not + // warn about unused argument on them. Grammar here is a bit odd, how about: // Claim all arguments that c

[PATCH] D26564: Use PIC relocation mode by default for PowerPC64 ELF

2016-12-13 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D26564 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

2017-04-19 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. As a note: As follow-up work, we might want to extend Clang to add TBAA metadata to allocas and lifetime.start intrinsics so that the instrumentation pass can mark the types of memory upon declaration/construction instead of only upon first access. https://reviews.llv

[PATCH] D31885: Remove TBAA information from LValues representing union members

2017-04-19 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. > There was a deliberate decision to make TBAA conservative about type punning > in LLVM because, in practice, the strict form of TBAA you think we should > follow has proven to be a failed optimization paradigm: it just leads users > to globally disable TBAA, which is

[PATCH] D31885: Remove TBAA information from LValues representing union members

2017-04-19 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D31885#730728, @kparzysz wrote: > In https://reviews.llvm.org/D31885#730038, @hfinkel wrote: > > > I'm somewhat concerned that this patch is quadratic in the AST. > > > I'd be happy to address this, but I'm not sure how. Memoizing results coul

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

2017-04-19 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D32199#730716, @filcab wrote: > Missing a `sanitizer-ld.c` test for freebsd. Thanks for pointing this out. I'm going to remove the freebsd change for now. I suspect it works, but I've not tested it yet. Comment at: includ

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

2017-04-19 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel updated this revision to Diff 95829. hfinkel added a comment. Updated per review comments (use only no_sanitize("tbaa") instead of adding no_sanitize_tbaa and don't touch freebsd for now). https://reviews.llvm.org/D32199 Files: include/clang/Basic/Sanitizers.def include/clang/Drive

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

2017-04-19 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D32199#731144, @rsmith wrote: > ... > I would also expect that we will extend this in future to assign types to > storage even in cases where there is no store (for instance, we should be > able to catch `float f() { int n; return *(float*

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

2017-04-19 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D32199#731249, @hfinkel wrote: > In https://reviews.llvm.org/D32199#731144, @rsmith wrote: > > > > > > ... > > > I would also expect that we will extend this in future to assign types to > > storage even in cases where there is no store (for i

[PATCH] D31885: Remove TBAA information from LValues representing union members

2017-04-19 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D31885#730853, @hfinkel wrote: > ... > > >> (And the nonsensicality of the standard very much continues. The code that >> we've all agreed is obviously being miscompiled here is still a strict >> violation of the "improved" union rules i

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

2017-04-19 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D32199#731332, @rsmith wrote: > > ! In https://reviews.llvm.org/D32199#731252, @hfinkel wrote: > > > >> How about renaming this to something more like `-fsanitize=type`? > > > > I'm fine with that. Do you like TypeSanitizer or TypeAccessSanti

[PATCH] D32260: [TBAA] Vector types should (only) alias with their element types

2017-04-19 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel created this revision. Herald added a subscriber: mcrosier. Currently, all of our builtin vector types are equivalent to char for TBAA purposes. It would be useful to make this less conservative. This patch makes vector types equivalent to their element types for type-aliasing purposes.

[PATCH] D31561: cmath: Skip Libc for integral types in isinf, etc.

2017-04-19 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D31561#731498, @dexonsmith wrote: > Marshall, that's what I assumed originally, but I figured Hal had some > non-standard-but-worth-supporting use case in mind. In this case, future-proofing and mathematical precision seemed aligned. I susp

[PATCH] D32260: [TBAA] Vector types should (only) alias with their element types

2017-04-19 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel abandoned this revision. hfinkel added a comment. @rjmccall said, on this topic, in https://reviews.llvm.org/D31885: > The root problem there is that the design of vector types and vector > interfaces is generally quite bad; you cannot rely on things like vectors > being stored with an

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

2017-04-20 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D32199#731472, @rsmith wrote: > > As I've currently implemented it, both reads and writes set the type of > > previously-unknown storage, and after that it says fixed (unless you memcpy > > to it, memset it, or its lifetime ends (the type get

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

2017-04-20 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D32199#731472, @rsmith wrote: > > As I've currently implemented it, both reads and writes set the type of > > previously-unknown storage, and after that it says fixed (unless you memcpy > > to it, memset it, or its lifetime ends (the type get

[PATCH] D32301: Don't pass FPOpFusion::Strict to the backend

2017-04-20 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D32301 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

2017-04-20 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D32199#732382, @rjmccall wrote: > If you're going to try to enforce the declared type of memory, you'll also > need something like the C effective type rule to handle char buffers in C++. > As far as I can tell, it's not actually legal under

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

2017-04-21 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D32199#732737, @rsmith wrote: > In https://reviews.llvm.org/D32199#732189, @hfinkel wrote: > > > In https://reviews.llvm.org/D32199#731472, @rsmith wrote: > > > > > 1. C's "effective type" rule allows writes to set the type pretty much > > > u

[PATCH] D32199: [TySan] A Type Sanitizer (Clang)

2017-04-21 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel updated this revision to Diff 96135. hfinkel retitled this revision from "[TBAASan] A TBAA Sanitizer (Clang)" to "[TySan] A Type Sanitizer (Clang)". hfinkel edited the summary of this revision. hfinkel added a comment. Rename TBAASanitizer -> TypeSanitizer https://reviews.llvm.org/D3219

[PATCH] D32199: [TySan] A Type Sanitizer (Clang)

2017-04-21 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel updated this revision to Diff 96264. hfinkel added a comment. Output metadata to provide the types of globals (similar to how Clang marks globals for asan). https://reviews.llvm.org/D32199 Files: include/clang/Basic/Sanitizers.def include/clang/Driver/SanitizerArgs.h lib/CodeGen/

[PATCH] D29654: [OpenMP] Integrate OpenMP target region cubin into host binary

2017-05-09 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added inline comments. Comment at: lib/Driver/ToolChains/Cuda.cpp:388 + + // add paths specified in LIBRARY_PATH environment variable as -L options. + addDirectoryList(Args, CmdArgs, "-L", "LIBRARY_PATH"); Comment sentences should start with a capital l

[PATCH] D33328: [CodeGen] Pessimize aliasing for union members (and may-alias) objects

2017-05-23 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added inline comments. Comment at: test/CodeGen/union-tbaa1.c:44 +// CHECK: ![[CHAR:[0-9]+]] = !{!"omnipotent char" +// CHECK: ![[OCPATH]] = !{![[CHAR]] Please check the path offset? Comment at: test/CodeGenCXX/union-tbaa2.cpp:45 +// CH

[PATCH] D33328: [CodeGen] Pessimize aliasing for union members (and may-alias) objects

2017-05-23 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added inline comments. Comment at: test/CodeGen/union-tbaa1.c:44 +// CHECK: ![[CHAR:[0-9]+]] = !{!"omnipotent char" +// CHECK: ![[OCPATH]] = !{![[CHAR]] kparzysz wrote: > hfinkel wrote: > > Please check the path offset? > What needs to be checked exactly?

<    1   2   3   4