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
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
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:
> > >
> > > >
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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*
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
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
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
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.
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
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
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
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
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
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
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
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
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/
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
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
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?
301 - 334 of 334 matches
Mail list logo