[PATCH] D24933: Enable configuration files in clang

hfinkel added inline comments. Comment at: include/clang/Driver/Driver.h:219 + /// This number may be smaller that \c NumConfigOptions if some options + /// requires separate arguments. + unsigned NumConfigArgs; requires -> require Comment

[PATCH] D24933: Enable configuration files in clang

hfinkel added inline comments. Comment at: lib/Driver/Driver.cpp:172 +NumConfigArgs = static_cast(NumCfgArgs); + } + sepavloff wrote: > bruno wrote: > > If `NumCfgArgs == 0` it would be nice to warn that the config file is empty? > Not sure if it makes

[PATCH] D29778: Declare lgamma library builtins as never being const

hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D29778 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

hfinkel added a comment. In https://reviews.llvm.org/D28404#673260, @mehdi_amini wrote: > Ping :) To clarify my understanding of this thread, it seems like there are three ways forward here: 1. To have -O0 add optnone to the generated functions (enabling some degree of lack of optimization

[PATCH] D29750: [PPC] Enable -fomit-frame-pointer by default for PPC

hfinkel added a comment. We should have regression tests for this. Maybe update test/Driver/frame-pointer-elim.c? https://reviews.llvm.org/D29750 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D28543: Elliminates uninitialized warning for volitile varibles.

hfinkel added a comment. What's the motivation for this? The placement of a local volatile variable is still under the compiler's direction, and unless the address escapes, we still assume we can reason about its aliasing (and, thus, whether or not it is initialized).

[PATCH] D30009: Add support for '#pragma clang attribute'

hfinkel added a comment. In https://reviews.llvm.org/D30009#678890, @arphaman wrote: > In https://reviews.llvm.org/D30009#678091, @hfinkel wrote: > > > I don't understand why it only supports some attributes. Is there some > > handling that needs to take place (I don't see anything obvious in

[PATCH] D30009: Add support for '#pragma clang attribute'

hfinkel added a comment. I don't understand why it only supports some attributes. Is there some handling that needs to take place (I don't see anything obvious in this patch)? If most attributes will "just work", I'd much rather opt-out the few exceptions (which we can then explicitly

[PATCH] D28845: Prototype of modules codegen

hfinkel added a comment. Can you provide a high-level description of what you're trying to accomplish and the usage model? https://reviews.llvm.org/D28845 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30087: [Driver] Unify linking of OpenMP runtime

hfinkel added inline comments. Comment at: lib/Driver/Tools.cpp:10334 - if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ, - options::OPT_fno_openmp, false)) { + // FIXME: Exclude this for platforms with libgomp that don't require +

[PATCH] D23765: Fix for clang PR 29087

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=79354#toc Repository: rL LLVM

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

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

[PATCH] D24933: Enable configuration files in clang

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

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

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

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

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

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

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

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

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

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

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] D27872: [APFloat] Switch from (PPCDoubleDoubleImpl, IEEEdouble) layout to (IEEEdouble, IEEEdouble)

hfinkel added a comment. > (IEEEdouble, IEEEdouble) -> (uint64_t, uint64_t) -> PPCDoubleDoubleImpl, then > run the old algorithm. We need to document in the code what is going on here and why it works. Just looking at a bunch of code like this: if (Semantics == ) { APFloat

[PATCH] D24688: Introduce "hosted" module flag.

hfinkel added a comment. In https://reviews.llvm.org/D24688#639158, @jyknight wrote: > Since this requires everyone generating llvm IR to add this module attribute > for optimal codegen, it should be documented in release notes and the > LangRef, I think. And, for something like this, here:

[PATCH] D23934: Add a -ffixed-date-time= flag that sets the initial value of __DATE__, __TIME__, __TIMESTAMP__

hfinkel added a comment. In https://reviews.llvm.org/D23934#632216, @emaste wrote: > Please also accept `SOURCE_DATE_EPOCH` set in the environment -- see > https://reproducible-builds.org/specs/source-date-epoch/ It looks like there's reasonable adoption of this:

[PATCH] D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin

hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM. This seems consistent with what GCC does. On x86 it also sets __GCC_ATOMIC_LLONG_LOCK_FREE to 2. https://reviews.llvm.org/D28213 ___

[PATCH] D23934: Add a -ffixed-date-time= flag that sets the initial value of __DATE__, __TIME__, __TIMESTAMP__

hfinkel added a comment. In https://reviews.llvm.org/D23934#631656, @ed wrote: > I'd be interested in seeing a feature like this appearing. I agree. Comment at: lib/Driver/Tools.cpp:4687 +CmdArgs.push_back(Args.MakeArgString("-ffixed-date-time=" + DateTime)); + } +

[PATCH] D31276: Add #pragma clang fast_math

hfinkel added a comment. High-level comment ;) #pragma clang fast_math contract_fast(on) This seems a bit unfortunate because 'fast' appears twice? How are we planning on naming the other fast-math flags? Maybe we should just name it: #pragma clang math constract_fast(on) or #pragma

[PATCH] D31276: Add #pragma clang fast_math

hfinkel added a comment. In https://reviews.llvm.org/D31276#708993, @anemet wrote: > In https://reviews.llvm.org/D31276#708976, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D31276#708968, @anemet wrote: > > > > > Thanks very much, Aaron! It would be great if you could also look at

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

hfinkel added a comment. In https://reviews.llvm.org/D30920#703305, @mehdi_amini wrote: > The fundamental difference, is that Os/Oz especially are treated as > `optimizations directive` that are independent of the pass pipeline: > instructing that "loop unroll should not increase size" is

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

hfinkel added a comment. In https://reviews.llvm.org/D30920#710329, @mehdi_amini wrote: > In https://reviews.llvm.org/D30920#710209, @hfinkel wrote: > > > Yes, we should do this. I don't understand why this is tricky. Actually, I > > think having these kinds of decisions explicit in the code of

[PATCH] D31276: Add #pragma clang fast_math

hfinkel added a comment. In https://reviews.llvm.org/D31276#709111, @anemet wrote: > In https://reviews.llvm.org/D31276#708992, @hfinkel wrote: > > > High-level comment ;) > > > > #pragma clang fast_math contract_fast(on) > > > > > > This seems a bit unfortunate because 'fast' appears

[PATCH] D30806: [nonnull] Teach Clang to attach the nonnull LLVM attribute to declarations and calls instead of just definitions, and then teach it to *not* attach such attributes even if the source c

hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D30806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

hfinkel added a comment. In https://reviews.llvm.org/D30923#700780, @rnk wrote: > In https://reviews.llvm.org/D30923#700708, @hfinkel wrote: > > > In https://reviews.llvm.org/D30923#700696, @rnk wrote: > > > > > Do you think it's worth indicating that the error can be suppressed with > > > an

[PATCH] D30898: Add new -fverbose-asm that enables source interleaving

hfinkel added a comment. > Note that in contrast to the original suggestion -fsource-asm here we use the > preferred -fverbose-asm. Basically explicitly saying -fverbose-asm in the > command line enables a minimum amount of debugging, so in AsmPrinter we can > use it to print the source code.

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

hfinkel added a comment. In https://reviews.llvm.org/D30920#700741, @mehdi_amini wrote: > In https://reviews.llvm.org/D30920#700574, @hfinkel wrote: > > > In https://reviews.llvm.org/D30920#700557, @mehdi_amini wrote: > > > > > In https://reviews.llvm.org/D30920#700433, @tejohnson wrote: > > > >

[PATCH] D30806: [nonnull] Teach Clang to attach the nonnull LLVM attribute to declarations and calls instead of just definitions, and then teach it to *not* attach such attributes even if the source c

hfinkel added inline comments. Comment at: include/clang/AST/ASTContext.h:1868 + bool *OverrideNonnullReturn = nullptr, + unsigned *OverrideNonnullArgs = nullptr, unsigned *IntegerConstantArgs =

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

hfinkel added a comment. In https://reviews.llvm.org/D30920#703083, @mehdi_amini wrote: > In https://reviews.llvm.org/D30920#703082, @hfinkel wrote: > > > In https://reviews.llvm.org/D30920#700741, @mehdi_amini wrote: > > > > > Yes, the issue is only about how the driver accepts Os for the link

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

hfinkel added a comment. In https://reviews.llvm.org/D30415#703398, @echristo wrote: > Different suggestion: > > Remove the faltivec option. Even gcc doesn't support it anymore afaict. What are you suggesting? Always having the language extensions on? Or explicitly tying the language

[PATCH] D30806: [nonnull] Teach Clang to attach the nonnull LLVM attribute to declarations and calls instead of just definitions, and then teach it to *not* attach such attributes even if the source c

hfinkel added inline comments. Comment at: include/clang/AST/ASTContext.h:1865 /// arguments to the builtin that are required to be integer constant /// expressions. QualType GetBuiltinType(unsigned ID, GetBuiltinTypeError , Please add some description

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

hfinkel added inline comments. Comment at: libcxx/include/math.h:354 inline _LIBCPP_INLINE_VISIBILITY -typename std::enable_if::value, bool>::type +typename std::enable_if::value, bool>::type signbit(_A1 __lcpp_x) _NOEXCEPT

[PATCH] D31167: Use FPContractModeKind universally

hfinkel added inline comments. Comment at: cfe/trunk/lib/Frontend/CompilerInvocation.cpp:1638 Opts.LaxVectorConversions = 0; -Opts.DefaultFPContract = 1; +Opts.setDefaultFPContractMode(LangOptions::FPC_On); Opts.NativeHalfType = 1; yaxunl

[PATCH] D31167: Use FPContractModeKind universally

hfinkel added inline comments. Comment at: cfe/trunk/lib/Frontend/CompilerInvocation.cpp:1638 Opts.LaxVectorConversions = 0; -Opts.DefaultFPContract = 1; +Opts.setDefaultFPContractMode(LangOptions::FPC_On); Opts.NativeHalfType = 1; Looks like

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

hfinkel added a subscriber: mclow.lists. hfinkel added a comment. I'm happy with this now. @EricWF , @mclow.lists , thoughts? https://reviews.llvm.org/D31561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D31167: Use FPContractModeKind universally

hfinkel added inline comments. Comment at: cfe/trunk/include/clang/Basic/LangOptions.def:220 +/// \brief FP_CONTRACT mode (on/off/fast). +ENUM_LANGOPT(DefaultFPContractMode, FPContractModeKind, 2, FPC_Off, "FP contraction type") LANGOPT(NoBitFieldTypeAlign , 1, 0, "bit-field

[PATCH] D31167: Use FPContractModeKind universally

hfinkel added inline comments. Comment at: cfe/trunk/include/clang/Basic/LangOptions.def:220 +/// \brief FP_CONTRACT mode (on/off/fast). +ENUM_LANGOPT(DefaultFPContractMode, FPContractModeKind, 2, FPC_Off, "FP contraction type") LANGOPT(NoBitFieldTypeAlign , 1, 0, "bit-field

[PATCH] D31167: Use FPContractModeKind universally

hfinkel added inline comments. Comment at: cfe/trunk/include/clang/Basic/LangOptions.def:220 +/// \brief FP_CONTRACT mode (on/off/fast). +ENUM_LANGOPT(DefaultFPContractMode, FPContractModeKind, 2, FPC_Off, "FP contraction type") LANGOPT(NoBitFieldTypeAlign , 1, 0, "bit-field

[PATCH] D31413: [libc++] Use __attribute__((init_priority(101))) to ensure streams get initialized early

hfinkel added a comment. In https://reviews.llvm.org/D31413#712013, @aaron.ballman wrote: > I'm not certain of a good way to test it, but I have a question about the > value you picked for `init_priority`. My understanding of the values starting > from 101 is that 1-100 are reserved for

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

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 > > >

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

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

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

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

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

hfinkel added a comment. I'm not sure this is the right way to do this; I suspect we're lumping together a bunch of different bugs: 1. vector types need to have tbaa which makes them alias with their element types [to be clear, as vector types are an implementation extension, this is our

[PATCH] D31167: Use FPContractModeKind universally

hfinkel added inline comments. Comment at: cfe/trunk/include/clang/Basic/LangOptions.def:220 +/// \brief FP_CONTRACT mode (on/off/fast). +ENUM_LANGOPT(DefaultFPContractMode, FPContractModeKind, 2, FPC_Off, "FP contraction type") LANGOPT(NoBitFieldTypeAlign , 1, 0, "bit-field

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

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

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

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

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

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

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

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

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

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

hfinkel added a comment. In https://reviews.llvm.org/D31885#727307, @kparzysz wrote: > This is not meant as a fine-grained solution to the TBAA problem, but a > temporary fix for generating wrong information. Just yesterday I helped > diagnose yet another problem related to this, so this

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

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:

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

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

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

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

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

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

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

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)

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

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

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] D32199: [TBAASan] A TBAA Sanitizer (Clang)

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

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

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

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

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.

[PATCH] D12689: [libc++][static linking] std streams are not initialized prior to their use in static object constructors

hfinkel added a comment. In https://reviews.llvm.org/D12689#515120, @eastig wrote: > The issue is reported as: > https://llvm.org/bugs/show_bug.cgi?id=28954 We do need to fix this bug; I posted to the PR so we can discuss approach there. https://reviews.llvm.org/D12689

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

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

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

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

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

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

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

hfinkel added a comment. In https://reviews.llvm.org/D30923#700696, @rnk wrote: > In https://reviews.llvm.org/D30923#700626, @hfinkel wrote: > > > In https://reviews.llvm.org/D30923#700620, @thakis wrote: > > > > > Maybe it should have some "to suppress the warning, do X" fixit? > > > > > > I

[PATCH] D29750: [PPC] Enable -fomit-frame-pointer by default for PPC

hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D29750 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

hfinkel added a comment. > I'm not sure about the solution to #2, because i thought there were very > specific points in time at which the effective type could change. I think this is a key point. I'm not sure that there are specific points that the frontend can deduce: union U { int i;

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

hfinkel added a comment. In https://reviews.llvm.org/D31885#727371, @efriedma wrote: > In https://reviews.llvm.org/D31885#727167, @hfinkel wrote: > > > I'm not sure this is the right way to do this; I suspect we're lumping > > together a bunch of different bugs: > > > > 1. vector types need to

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

hfinkel created this revision. Herald added subscribers: mcrosier, emaste. This patch introduces the runtime components of a TBAA sanitizer: a sanitizer for type-based aliasing violations. C/C++ have type-based aliasing rules, and LLVM's optimizer can exploit these given TBAA metadata added

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

hfinkel added a comment. To be clear, I'm find with disabling union tbaa until we can fix things for real. I'm somewhat concerned that this patch is quadratic in the AST. FWIW, I tried just setting TBAAPath = true in EmitLValueForField and then returning nullptr from

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

hfinkel added a comment. In https://reviews.llvm.org/D32199#828526, @rjmccall wrote: > Has this proposal run aground? I'm going back over some old patches that > I've been CC'ed on and trying to make sure they're not blocking on my review. I need to rebase these now that we've "fixed" the

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

hfinkel updated this revision to Diff 09. hfinkel added a comment. Rebased. https://reviews.llvm.org/D32199 Files: include/clang/Basic/Sanitizers.def include/clang/Driver/SanitizerArgs.h lib/CodeGen/BackendUtil.cpp lib/CodeGen/CGDecl.cpp lib/CodeGen/CGDeclCXX.cpp

[PATCH] D9403: llvm.noalias - Clang CodeGen for local restrict-qualified pointers

hfinkel added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1291 +// where the pointer to the local variable is the key in the map. +void CodeGenFunction::EmitAutoVarNoAlias(const AutoVarEmission ) { + assert(emission.Variable && "emission was not valid!");

[PATCH] D9403: llvm.noalias - Clang CodeGen for local restrict-qualified pointers

hfinkel updated this revision to Diff 29. hfinkel added a comment. Herald added a subscriber: javed.absar. Rebased and addressing review comments. https://reviews.llvm.org/D9403 Files: lib/CodeGen/CGDecl.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGStmt.cpp

[PATCH] D22189: llvm.noalias - Clang CodeGen - check restrict variable map only for restrict-qualified lvalues

hfinkel updated this revision to Diff 30. hfinkel added a comment. Herald added a subscriber: jholewinski. Rebased. https://reviews.llvm.org/D22189 Files: lib/CodeGen/CGDeclCXX.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp

[PATCH] D29660: [OpenMP] Add flag for overwriting default PTX version for OpenMP targets

hfinkel added a comment. In https://reviews.llvm.org/D29660#838150, @gtbercea wrote: > First of all, I apologize if I've upset you with my previous post. I am > actively working on understanding what is causing these issues. It is not my > intention to write tests that work on local

[PATCH] D36537: [OpenMP] Enable executable lookup into driver directory.

hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. I think this is fine, but please add a comment explaining that you're doing this to find (e.g., to find the offload bundler, etc.). Repository: rL LLVM https://reviews.llvm.org/D36537

[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

hfinkel added a comment. This is much closer to what I had in mind, thanks! Now I think we're in a position to make this work for more than just the CUDA target. It looks like the added code is now: 1. Remove -march from the translated arguments (because any existing -march would apply only

[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

hfinkel added inline comments. Comment at: lib/Driver/ToolChains/Cuda.cpp:474 +for (StringRef Opt : OptList) { + AddMArchOption(DAL, Opts, Opt); +} gtbercea wrote: > hfinkel wrote: > > gtbercea wrote: > > > hfinkel wrote: > > > > Shouldn't you be

[PATCH] D34158: For standards compatibility, preinclude if the file is available

hfinkel added a comment. In https://reviews.llvm.org/D34158#803752, @jyknight wrote: > This version is still disabling upon -nostdinc, which doesn't make sense to > me. I agree. If I pass -nostdinc, so that I then arrange include paths for my own libc headers, I'd then like to pick up the

[PATCH] D29647: [OpenMP] Extend CLANG target options with device offloading kind.

hfinkel added inline comments. Comment at: test/Driver/openmp-offload.c:614 +/// Check -march propagates compute capability to device offloading toolchain. +// RUN: %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -save-temps -no-canonical-prefixes

[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

hfinkel added a comment. In https://reviews.llvm.org/D34784#795871, @gtbercea wrote: > In https://reviews.llvm.org/D34784#795367, @hfinkel wrote: > > > In https://reviews.llvm.org/D34784#795353, @gtbercea wrote: > > > > > In https://reviews.llvm.org/D34784#795287, @hfinkel wrote: > > > > > > >

[PATCH] D34846: Remove Clang support for '-fvectorize-slp-aggressive' which used LLVM's basic block vectorizer. This vectorizer has had no known users for many, many years and is completely surpassed

hfinkel accepted this revision. hfinkel added a comment. LGTM, but you need to get the: def fslp_vectorize_aggressive : Flag<["-"], "fslp-vectorize-aggressive">, Group, HelpText<"Enable the BB vectorization passes">; def fno_slp_vectorize_aggressive : Flag<["-"],

[PATCH] D34846: Remove Clang support for '-fvectorize-slp-aggressive' which used LLVM's basic block vectorizer. This vectorizer has had no known users for many, many years and is completely surpassed

hfinkel added a comment. In https://reviews.llvm.org/D34846#796102, @rsmith wrote: > Removing a `-cc1` flag is OK: we explicitly tell people that the `-cc1` > interface is subject to change without notice. I'm assuming we can remove the regular flag as well. They'll just get an "unused

[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

hfinkel added a comment. In https://reviews.llvm.org/D34784#795980, @gtbercea wrote: > In https://reviews.llvm.org/D34784#795934, @hfinkel wrote: > > > In https://reviews.llvm.org/D34784#795871, @gtbercea wrote: > > > > > In https://reviews.llvm.org/D34784#795367, @hfinkel wrote: > > > > > > >

[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

hfinkel added a comment. ... > Okay, good, this is exactly where I was going when I said I was worried about generalization. -march seems like one of many flags I might want to pass to the target compilation. Moreover, it doesn't seem special in what regard.

[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

hfinkel added a comment. What happens if you have multiple targets? Maybe this should be -fopenmp-targets-arch=foo,bar,whatever? Once this all lands, please make sure that you add additional test cases here. Make sure that the arch is passed through to the ptx and cuda tools as it should be.

[PATCH] D29647: [OpenMP] Extend CLANG target options with device offloading kind.

hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM Repository: rL LLVM https://reviews.llvm.org/D29647 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

hfinkel added a comment. In https://reviews.llvm.org/D34784#795353, @gtbercea wrote: > In https://reviews.llvm.org/D34784#795287, @hfinkel wrote: > > > What happens if you have multiple targets? Maybe this should be > > -fopenmp-targets-arch=foo,bar,whatever? > > > > Once this all lands, please

[PATCH] D29647: [OpenMP] Extend CLANG target options with device offloading kind.

hfinkel added a comment. In https://reviews.llvm.org/D29647#795271, @hfinkel wrote: > LGTM When you commit this, please make sure to mention in the commit message that the test cases will be associated with follow-up commits. Repository: rL LLVM https://reviews.llvm.org/D29647

[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

hfinkel added inline comments. Comment at: include/clang/Driver/Options.td:453 +def Xopenmp_target : Separate<["-"], "Xopenmp-target">, + HelpText<"Pass arguments to target offloading toolchain.">; +def Xopenmp_target_EQ : JoinedAndSeparate<["-"], "Xopenmp-target=">,

[PATCH] D34928: Add docs for -foptimization-record-file=

hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D34928 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29647: [OpenMP] Extend CLANG target options with device offloading kind.

hfinkel added inline comments. Comment at: lib/Driver/ToolChains/Cuda.cpp:435 + +// TODO: get the compute capability from offloading arguments when not +// using the default compute capability of sm_20. gtbercea wrote: > hfinkel wrote: > > gtbercea

[PATCH] D29647: [OpenMP] Extend CLANG target options with device offloading kind.

hfinkel added inline comments. Comment at: lib/Driver/ToolChains/Cuda.cpp:435 + +// TODO: get the compute capability from offloading arguments when not +// using the default compute capability of sm_20. gtbercea wrote: > hfinkel wrote: > > Why is this a

[PATCH] D29647: [OpenMP] Extend CLANG target options with device offloading kind.

hfinkel added inline comments. Comment at: test/Driver/openmp-offload.c:614 +/// Check -march propagates compute capability to device offloading toolchain. +// RUN: %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -save-temps -no-canonical-prefixes

  1   2   3   4   >