[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

efriedma added a comment. I mean, which of the callers of startFrontendTimer() is calling it with a pointer to std::declval()? https://reviews.llvm.org/D47196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

efriedma added a comment. Oh, oops, I should have spotted that. No, the point of this was to separate the timing infrastructure, not to change the behavior of -ftime-report. Should be a one-line change to add "llvm::TimePassesIsEnabled = TimePasses" back to BackendConsumer::BackendConsumer. (

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

efriedma added inline comments. Comment at: lib/Driver/ToolChains/Arch/AArch64.cpp:266 + const bool HasV83a = (std::find(ItBegin, ItEnd, "+v8.3a") != ItEnd); + const bool HasV84a = (std::find(ItBegin, ItEnd, "+v8.4a") != ItEnd); + const bool HasV85a = (std::find(It

[PATCH] D50507: [CodeGen][ARM] Coerce FP16 vectors to integer vectors when needed

efriedma added a comment. Do we need to check for homogeneous aggregates of half vectors somewhere? https://reviews.llvm.org/D50507 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature

efriedma added a comment. Somewhere around CompilerInvocation::CreateFromArgs is probably appropriate. Repository: rL LLVM https://reviews.llvm.org/D45619 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D50507: [CodeGen][ARM] Coerce FP16 vectors to integer vectors when needed

efriedma added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:5788 + llvm::Type *Ty = llvm::ArrayType::get(NewVecTy, Members); + return ABIArgInfo::getDirect(Ty, 0, nullptr, false); +} Do we need equivalent code in classifyRetur

[PATCH] D50683: [Android] Set NewAlign for 64-bit Android to 8 bytes

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/Basic/TargetInfo.cpp:72 +// For 64-bit Android, alignment is 8 bytes for allocations <= 8 bytes. +NewAlign = (Triple.isArch64Bit() || Triple.

[PATCH] D50683: [Android] Set NewAlign for 64-bit Android to 8 bytes

efriedma added a comment. The C++ standard just says "An integer literal of type std::size_t whose value is the alignment guaranteed by a call to operator new(std::size_t) or operator new[](std::size_t)." I would take that in the obvious sense, that any pointer returned by operator new must ha

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

efriedma added a comment. > :start: means the timer was started In some cases, the ChildTime is already non-zero at the "start" point; what does that mean? > ActOnFunctionDeclarator:start:._exception: That's weird... DeclarationName::print should generally return something ASCII; do you k

[PATCH] D51011: [Preprocessor] raise gcc compatibility macros.

efriedma added a comment. I'd suggest something like this, if you really need to detect the compiler: #if defined(__clang__) // clang #elif defined(__INTEL_COMPILER) // icc #elif defined(__GNUC__) // gcc #else #error "Unknown compiler" #endif > Regarding the glibc headers, do

[PATCH] D46535: Correct warning on Float->Integer conversions.

efriedma added a comment. The check for whether an input is "out of range" doesn't handle fractions correctly. Testcase: int a() { return 2147483647.5; } unsigned b() { return -.5; } Repository: rC Clang https://reviews.llvm.org/D46535 ___

[PATCH] D46349: [X86] Mark builtins 'const' where possible

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. I'm pretty sure this has zero visible effect, but I guess it makes sense as documentation. LGTM. https://reviews.llvm.org/D46349 ___ cfe-co

[PATCH] D46112: Allow _Atomic to be specified on incomplete types

efriedma added a comment. I think the request was that we check that a type is trivially copyable when we perform an atomic operation? I don't see the code for that anywhere. Also needs some test coverage for atomic operations which aren't calls, like "typedef struct S S; void f(_Atomic S *s,

[PATCH] D45616: [X86] Lower _mm[256|512]_cmp[.]_mask intrinsics to native llvm IR

efriedma added a comment. There is no difference between "signalling" and "non-signalling" unless you're using "#pragma STDC FENV_ACCESS", which is currently not supported. Presumably the work to implement that will include some LLVM IR intrinsic which can encode the difference, but for now we

[PATCH] D46822: [RISCV] Add driver for riscv32-unknown-elf baremetal target

efriedma added a comment. Could you include some documentation for how to construct a baremetal environment like the one this code expects? It's not clear what exactly you expect to be installed where. Repository: rC Clang https://reviews.llvm.org/D46822

[PATCH] D46863: [X86] Use __builtin_convertvector to implement some of the packed integer to packed flow conversion intrinsics.

efriedma added a comment. > I'm concerned about constant folding not taking into account runtime rounding > mode Changing the rounding mode is UB without FENV_ACCESS. And with FENV_ACCESS, __builtin_convertvector should lower to @llvm.experimental.constrained.sitofp etc., which won't constant

[PATCH] D46822: [RISCV] Add driver for riscv32-unknown-elf baremetal target

efriedma added a comment. Just a brief comment in the code explaining that would be fine. Repository: rC Clang https://reviews.llvm.org/D46822 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

[PATCH] D46665: [Itanium] Emit type info names with external linkage.

efriedma added a comment. The only difference between weak_odr and linkonce_odr is that the LLVM optimizers can discard linkonce_odr globals. From your description, you want to remove the odr-ness, by changing the linkage to "linkonce", I think? That said, I don't think the usage here violates

[PATCH] D46665: [Itanium] Emit type info names with external linkage.

efriedma added a comment. What exactly is the asan odr checker actually checking for? The distinction between common/linkonce_odr/linkonce/weak_odr/weak only affects IR optimizers, not code generation. Repository: rC Clang https://reviews.llvm.org/D46665

[PATCH] D47092: downgrade strong type info names to weak_odr linkage

efriedma added a comment. > This results in formal violations of LLVM's linkage rules I'm pretty sure this isn't actually a violation of LLVM's linkage rules as they are described in LangRef... at least, my understanding is that "equivalent" doesn't imply anything about linkage. (If this shoul

[PATCH] D45616: [X86] Lower _mm[256|512]_cmp[.]_mask intrinsics to native llvm IR

efriedma added a subscriber: andrew.w.kaylor. efriedma added a comment. I don't see any reason to exactly match the constant specified by the user, as long as the result is semantically equivalent. Once we have llvm.experimental.constrained.fcmp, this code should be updated to emit it; that wil

[PATCH] D36487: Emit section information for extern variables.

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D36487 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D37861: preserving #pragma clang assume_nonnull in preprocessed output

efriedma added a comment. Oops, sorry, lost track of it; I'll commit it today. https://reviews.llvm.org/D37861 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37861: preserving #pragma clang assume_nonnull in preprocessed output

This revision was automatically updated to reflect the committed changes. Closed by commit rL314364: [Preprocessor] Preserve #pragma clang assume_nonnull in preprocessed output (authored by efriedma). Changed prior to commit: https://reviews.llvm.org/D37861?vs=115838&id=116903#toc Repository:

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

efriedma added a comment. As far as I can see, there are three significant issues with the current -mgeneral-regs-only: 1. We don't correctly ignore inline asm clobbers for registers which aren't allocatable (https://bugs.llvm.org/show_bug.cgi?id=30792) 2. We don't diagnose calls which need vec

[PATCH] D38717: Patch to Bugzilla 31373

efriedma added a comment. Please make sure the title (subject line) for a patch reflects the actual contents of the change, as opposed to referring to Bugzilla; a lot of people skim cfe-commits, so you want to make sure interested reviewers find you patch. Needs a regression test to verify we g

[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

efriedma added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8612 +OtherT = OtherT->getAs()->getDecl()->getIntegerType(); + } + Why are you changing this here, as opposed to changing IntRange::forValueOfCanonicalType? Repository: rL LLVM https

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

efriedma added a comment. The gcc documentation says "GCC includes built-in versions of many of the functions in the standard C library. These functions come in two forms: one whose names start with the `__builtin_` prefix, and the other without. Both forms have the same type (including prototy

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

efriedma added a comment. I think you're understanding the semantics correctly. For r265521, look again at the implementation of llvm::checkUnaryFloatSignature; if "I.onlyReadsMemory()" is true, we somehow proved the call doesn't set errno (mostly likely because we know something about the tar

[PATCH] D51198: [LTO] Fix -save-temps with LTO and unnamed globals.

efriedma created this revision. efriedma added reviewers: tejohnson, tobiasvk. Herald added subscribers: dexonsmith, inglorion. If all LLVM passes are disabled, we can't emit a summary because there could be unnamed globals in the IR. Repository: rC Clang https://reviews.llvm.org/D51198 Fil

[PATCH] D51198: [LTO] Fix -save-temps with LTO and unnamed globals.

efriedma updated this revision to Diff 162308. efriedma added a comment. Fix new pass manager. Repository: rC Clang https://reviews.llvm.org/D51198 Files: lib/CodeGen/BackendUtil.cpp test/CodeGen/summary-index-unnamed-global.ll Index: test/CodeGen/summary-index-unnamed-global.ll ==

[PATCH] D51198: [LTO] Fix -save-temps with LTO and unnamed globals.

This revision was automatically updated to reflect the committed changes. Closed by commit rC340640: [LTO] Fix -save-temps with LTO and unnamed globals. (authored by efriedma, committed by ). Repository: rC Clang https://reviews.llvm.org/D51198 Files: lib/CodeGen/BackendUtil.cpp test/Code

[PATCH] D51265: Headers: fix collisions with .h files of other projects

efriedma edited reviewers, added: efriedma, rsmith, thakis; removed: eli.friedman, krememek, ddunbar, lattner. efriedma added a comment. What is this change actually solving? Even if the typedef is actually redundant, it shouldn't cause a compiler error: redundant typedefs are allowed in the l

[PATCH] D51190: [AttrDocs]: document gnu_inline function attribute

efriedma added a comment. Thanks for taking the time to write documentation. The phrase "C89 convention" is misleading; the original ISO C standard doesn't define the inline keyword at all. Maybe something along the lines of "GNU inline extension". And maybe mention it's the default with -std

[PATCH] D51265: Headers: fix collisions with .h files of other projects

efriedma added a comment. Can you give the text of the error messages? If you're seeing a "typedef redefinition with different types" error, libclang is getting confused about the target. And parsing code for the wrong target cannot work in general. Repository: rC Clang https://reviews.ll

[PATCH] D51416: [RTTI] Align rtti type string to prevent over-alignment

efriedma added a comment. Could we make CreateOrReplaceCXXRuntimeVariable take the alignment as an argument, so we can be sure we're consistently setting the alignment for these variables? This seems easy to mess up if we're scattering calls all over the place. Sort of orthogonal, but CreateO

[PATCH] D50229: +fp16fml feature for ARM and AArch64

efriedma added a comment. Do you expect that the regression tests will be affected by the TargetParser fixes? If not, I guess this is okay... but please add clear comments explaining which bits of code you expect to go away after the TargetParser rewrite. Repository: rC Clang https://revi

[PATCH] D51416: [RTTI] Align rtti types to prevent over-alignment

efriedma added a comment. getClassPointerAlignment is not really relevant here; that's the alignment of a pointer to an object with the type of the class. For the LLVM IR structs which don't have a corresponding clang type, you can probably just use DataLayout::getABITypeAlignment(). I think t

[PATCH] D51817: Do not use optimized atomic libcalls for misaligned atomics.

efriedma added inline comments. Comment at: lib/CodeGen/CGAtomic.cpp:949 case AtomicExpr::AO__opencl_atomic_compare_exchange_strong: case AtomicExpr::AO__atomic_load_n: case AtomicExpr::AO__atomic_store_n: Is there any particular reason to expect t

[PATCH] D51817: Do not use optimized atomic libcalls for misaligned atomics.

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/CodeGen/CGAtomic.cpp:949 case AtomicExpr::AO__opencl_atomic_compare_exchange_strong: case AtomicExpr::AO__atomic_load_n: case AtomicEx

[PATCH] D51416: [RTTI] Align rtti types to prevent over-alignment

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM with one change. Comment at: lib/CodeGen/ItaniumCXXABI.cpp:2730 llvm::GlobalVariable *GV = -CGM.CreateOrReplaceCXXRuntimeVariable(Name, Init->getType(), Linka

[PATCH] D50507: [CodeGen][ARM] Coerce FP16 vectors to integer vectors when needed

efriedma accepted this revision. efriedma added a comment. LGTM https://reviews.llvm.org/D50507 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46013: [ARM] Conform to AAPCS when passing overaligned composites as arguments

efriedma added inline comments. Comment at: lib/AST/ASTContext.cpp:2088 + default: +UnadjustedAlign = getTypeAlign(T); + } This "default" isn't right; there are a lot of non-canonical types which need to be handled here (which getTypeAlign handles, but thi

[PATCH] D45712: Diagnose invalid cv-qualifiers for friend decls.

efriedma added a comment. Herald added a subscriber: jfb. Ping Repository: rC Clang https://reviews.llvm.org/D45712 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46013: [ARM] Conform to AAPCS when passing overaligned composites as arguments

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D46013 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

efriedma added inline comments. Comment at: lib/Sema/SemaLambda.cpp:1447 +getFrontendFunctionTimeCtx()->startFrontendTimer( +{LSI.CallOperator, 0.0}); + } This seems sort of late? You're starting the timer after the body has already been parsed.

[PATCH] D46822: [RISCV] Add driver for riscv32-unknown-elf baremetal target

efriedma added a comment. Please don't commit patches on behalf of someone else if the author hasn't specifically requested it. There might be some issue which the author forgot to note on the review. Repository: rC Clang https://reviews.llvm.org/D46822 _

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

efriedma added a comment. I think there's something weird happening with your timers if they're showing half a second spent parsing or instantiating std::declval: it doesn't have a definition, anywhere (it's just a placeholder for template metaprogramming tricks). Comment at

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

efriedma added inline comments. Comment at: lib/Driver/ToolChains/Arch/AArch64.cpp:200 + // + // TODO: implement this logic in TargetParser. + Yes, this logic should be in TargetParser, not here. Trying to rewrite the target features afterwards is messy at be

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

efriedma added a comment. "0.0040" is four milliseconds? You're probably crediting time incorrectly, somehow. Can you tell which FrontendTimeRAII the time is coming from? https://reviews.llvm.org/D47196 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D45712: Diagnose invalid cv-qualifiers for friend decls.

This revision was automatically updated to reflect the committed changes. Closed by commit rL338931: Diagnose invalid cv-qualifiers for friend decls. (authored by efriedma, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D45712?vs=15470

[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag

efriedma added a comment. > This flag can then be used to build compiler-rt for RISCV32. Can you give a little more context for why this is necessary? As far as I know, compiler-rt generally supports building for targets which don't have __int128_t. (If all riscv targets are supposed to suppo

[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag

efriedma added a comment. So you want int128_t for compiler-rt itself, so you can use the soft-float implementation, but you want to make int128_t opt-in to avoid the possibility of someone getting a link error trying to link code built with clang against libgcc.a? That seems a little convolut

[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag

efriedma added inline comments. Comment at: include/clang/Driver/Options.td:843 +def fforce_enable_int128 : Flag<["-"], "fforce-enable-int128">, Group, + Flags<[CC1Option]>, HelpText<"Enable support for int128_t type">; We should have an inverse flag -fno-forc

[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Please post a follow-up to add this to the 7.0 release notes. Repository: rC Clang https://reviews.llvm.org/D43105 ___ cfe-commits m

[PATCH] D43540: [WebAssembly] Enable -Werror=strict-prototypes by default

efriedma added a comment. If someone is compiling C code that doesn't have undefined behavior, it should work; if it doesn't, that's a clear bug. (As far as I know, there shouldn't be any issues here, but if there are, file a bug and CC me.) WebAssembly is not the only platform where varargs a

[PATCH] D43423: [SimplifyCFG] Create flag to disable simplifyCFG.

efriedma added a comment. > FWIW, I'd very much prefer the details of the optimizer wouldn't be exposed > as frontend flags. Yes, definitely this. Frontend flags to control the optimizer should state an expected result, not just turn off some completely arbitrary set of transforms. Otherwise

[PATCH] D31972: Do not force the frame pointer by default for ARM EABI

efriedma added reviewers: t.p.northover, efriedma. efriedma added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:569 + if (Triple.getEnvironment() == llvm::Triple::EABI) { +switch (Triple.getArch()) { Specifically checking for "llvm::Triple::

[PATCH] D33774: [CodeGen] Make __attribute__(const) calls speculatable

efriedma added a comment. Consider something like this: define i32 @div(i32 %x, i32 %y) { entry: %div = sdiv i32 %x, %y ret i32 %div } We can mark this function readnone, but not speculatable: it doesn't read or write memory, but could exhibit undefined behavior. Consider another

[PATCH] D33765: Show correct column nr. when multi-byte utf8 chars are used.

efriedma added a comment. Correctly counting columns is a bit more complicated that that... for example, consider what happens if you replace `ideëen` with `idez̈en`. See https://stackoverflow.com/questions/3634627/how-to-know-the-preferred-display-width-in-columns-of-unicode-characters . ht

[PATCH] D31972: Do not force the frame pointer by default for ARM EABI

efriedma added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:569 + if (Triple.getEnvironment() == llvm::Triple::EABI) { +switch (Triple.getArch()) { chrib wrote: > efriedma wrote: > > Specifically checking for "llvm::Triple::EABI" is suspici

[PATCH] D33926: [clang] Remove double semicolons. NFC.

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. You don't need to ask for review for a trivial change like this. https://reviews.llvm.org/D33926 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D31972: Do not force the frame pointer by default for ARM EABI

efriedma added a comment. Please start a thread on cfe-dev about this; most developers don't read cfe-commits, and thinking about it a bit more, I'm not confident omitting frame pointers is really a good default. I would guess there's existing code which depends on frame pointers to come up wi

[PATCH] D33910: [ubsan] Detect invalid unsigned pointer index expression (clang)

efriedma added inline comments. Comment at: cfe/trunk/lib/CodeGen/CGExprScalar.cpp:2666 + bool isSigned = indexOperand->getType()->isSignedIntegerOrEnumerationType(); + bool mayHaveNegativeGEPIndex = isSigned || isSubtraction; + This logic doesn't look quite ri

[PATCH] D34121: [ubsan] Teach the pointer overflow check that "p - <= p" (PR33430)

efriedma added a comment. Looks okay, but I haven't been reviewing this series of patches closely, so I could be missing something. Comment at: lib/CodeGen/CGExprScalar.cpp:3974 ValidGEP = Builder.CreateAnd(PosOrZeroValid, NoOffsetOverflow); + } else if (!SignedIndices &

[PATCH] D34523: AST: mangle BlockDecls under MS ABI

efriedma added a comment. Patch is missing context. You have to use getBlockManglingNumber() for blocks which are externally visible; otherwise, the numbers won't be consistent in other translation units. Repository: rL LLVM https://reviews.llvm.org/D34523 ___

[PATCH] D34523: AST: mangle BlockDecls under MS ABI

efriedma added a comment. > @efriedma hmm...using getBlockManglingNumber causes the name to be > duplicated. Ill look into that. Have you looked at the Itanium mangling implementation? > However, wouldn't all the block invocation functions be defined and COMDAT'ed? IIRC, we always emit blocks

[PATCH] D34568: [Sema] Make BreakContinueFinder handle nested loops.

efriedma created this revision. We don't care about break or continue statements that aren't associated with the current loop, so make sure the visitor doesn't find them. Fixes https://bugs.llvm.org/show_bug.cgi?id=32648 . Repository: rL LLVM https://reviews.llvm.org/D34568 Files: lib/Sem

[PATCH] D34523: AST: mangle BlockDecls under MS ABI

efriedma added a comment. I think we just use "0" as a sentinel value to indicate the block doesn't need a mangling number. getLambdaManglingNumber works the same way? See CXXNameMangler::mangleUnqualifiedBlock in the Itanium mangler. Repository: rL LLVM https://reviews.llvm.org/D34523

[PATCH] D34523: AST: mangle BlockDecls under MS ABI

efriedma added a comment. > How do we end up in a state where the block invocation function is > referenced external to the TU? ODR allows certain definitions, like class definitions and inline function definitions, to be written in multiple translation units. See http://itanium-cxx-abi.git

[PATCH] D41149: [CodeGen] Specialize mixed-sign mul-with-overflow (fix PR34920)

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/CodeGen/CGBuiltin.cpp:912 + auto IntMax = + llvm::APInt::getMaxValue(ResultInfo.Width).zextOrSelf(Op1Info.Width); + llvm::Value *T

[PATCH] D41374: [Coverage] Fix use-after free in coverage emission

efriedma created this revision. efriedma added reviewers: vsk, davidxl. efriedma added a project: clang. Fixes regression from r320533. This fixes the undefined behavior, but I'm not sure it's really right... I think we end up with missing coverage for code in modules. Repository: rC Clang

[PATCH] D41374: [Coverage] Fix use-after free in coverage emission

This revision was automatically updated to reflect the committed changes. Closed by commit rL321052: [Coverage] Fix use-after free in coverage emission (authored by efriedma, committed by ). Changed prior to commit: https://reviews.llvm.org/D41374?vs=127440&id=127451#toc Repository: rL LLVM

[PATCH] D41421: Eliminate a magic number. NFC.

efriedma added a comment. Please use llvm::array_lengthof to compute the length of an array. Alternatively, there's an overload of PP.getSpelling which takes a SmallVector and returns a StringRef as a parameter; you could change this code to use it, which would remove the need for the check.

[PATCH] D41421: Eliminate a magic number. NFC.

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D41421 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D40698: [ubsan] Diagnose noreturn functions which return

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D40698 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D33563: Track whether a unary operation can overflow

efriedma added inline comments. Comment at: include/clang/AST/Expr.h:1728 + UnaryOperator(Expr *input, Opcode opc, QualType type, ExprValueKind VK, +ExprObjectKind OK, SourceLocation l, bool CanOverflow = false) + : Expr(UnaryOperatorClass, type, VK, OK, ---

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

efriedma added inline comments. Comment at: lib/Frontend/CompilerInvocation.cpp:2252 + TM.tm_isdst = -1; + mktime(&TM); + Opts.FixedDateTime = TM; Does using mktime like this depend on the local timezone? https://reviews.llvm.org/D23934 __

[PATCH] D41717: [CGBuiltin] Handle unsigned mul overflow properly (PR35750)

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D41717 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D41780: Preserve unknown STDC pragma through preprocessor

efriedma added a comment. If you move all the #pragma STDC handlers from the lexer to the parser, you might be able to avoid adding an explicit STDC handler in PrintPreprocessedOutput.cpp. Comment at: test/Preprocessor/pragma_unknown.c:32 #pragma STDC SO_GREAT // expected-w

[PATCH] D41780: Preserve unknown STDC pragma through preprocessor

efriedma added a comment. Should be safe, I think; currently, FENV_ACCESS and CX_LIMITED_RANGE have no effect, and when we do start supporting them, we'll probably want to handle them in the parser, like we do for FP_CONTRACT. Repository: rC Clang https://reviews.llvm.org/D41780

[PATCH] D41780: Preserve unknown STDC pragma through preprocessor

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D41780 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D33563: Track whether a unary operation can overflow

efriedma added inline comments. Comment at: test/Misc/ast-dump-stmt.c:66 + // CHECK:ImplicitCastExpr + // CHECK: DeclRefExpr{{.*}}'T2' 'int' +} aaron.ballman wrote: > efriedma wrote: > > What does it mean for bitwise complement to "overflow"? >

[PATCH] D33563: Track whether a unary operation can overflow

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D33563 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D41517: mmintrin.h documentation fixes and updates

efriedma added inline comments. Comment at: lib/Headers/mmintrin.h:55 /// -/// This intrinsic corresponds to the VMOVD / MOVD instruction. +/// This intrinsic corresponds to the MOVD instruction. /// craig.topper wrote: > kromanova wrote: > > I tried clang

[PATCH] D41516: emmintrin.h documentation fixes and updates

efriedma added inline comments. Comment at: cfe/trunk/lib/Headers/emmintrin.h:4683 /// -/// This intrinsic has no corresponding instruction. +/// This intrinsic corresponds to the MOVDQ2Q instruction. /// kromanova wrote: > kromanova wrote: > > I'm not sure a

[PATCH] D41516: emmintrin.h documentation fixes and updates

efriedma added inline comments. Comment at: cfe/trunk/lib/Headers/emmintrin.h:4683 /// -/// This intrinsic has no corresponding instruction. +/// This intrinsic corresponds to the MOVDQ2Q instruction. /// kromanova wrote: > efriedma wrote: > > kromanova wrote

[PATCH] D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26

efriedma added a comment. > as this patch is committed clang is broken (cannot use glibc headers) This patch was supposed to *fix* compatibility with the glibc headers. If it doesn't, we should clearly revert it... but we need to figure out what we need to do to be compatible first. From what

[PATCH] D43734: [RecordLayout] Don't align to non-power-of-2 sizes when using -mms-bitfields

efriedma added inline comments. Comment at: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp:1758 + Context.getTargetInfo().getTriple().isWindowsGNUEnvironment()) && +"Non PowerOf2 size outside of GNU mode"); +if (TypeSize > FieldAlign &&

[PATCH] D43734: [RecordLayout] Don't align to non-power-of-2 sizes when using -mms-bitfields

efriedma added inline comments. Comment at: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp:1758 + Context.getTargetInfo().getTriple().isWindowsGNUEnvironment()) && +"Non PowerOf2 size outside of GNU mode"); +if (TypeSize > FieldAlign &&

[PATCH] D43908: [RecordLayout] Only assert that fundamental type sizes are power of two on MSVC

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D43908 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D44223: [ms] Emit vtordisp initializers in a deterministic order.

efriedma added inline comments. Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1202 + SmallVector VBases(VBaseMap.begin(), VBaseMap.end()); + std::stable_sort(VBases.begin(), VBases.end(), + [](const VBaseEntry &a, const VBaseEntry &b) { Using st

[PATCH] D44582: [Builtins] Fix calling long double math functions on x86_64 mingw

efriedma added a comment. Can we just fix the bug in the backend, rather than trying to hack around it in clang? Repository: rC Clang https://reviews.llvm.org/D44582 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D44582: [Builtins] Fix calling long double math functions on x86_64 mingw

efriedma added a comment. The backend has code to generate SRet returns, which is used when TargetLowering::CanLowerReturn returns false. Probably a tiny change to X86CallingConv.td to use that codepath on Windows. Repository: rC Clang https://reviews.llvm.org/D44582 ___

[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

efriedma added a comment. The compiler shouldn't inline functions which call va_start, whether or not they're marked always_inline. That should work correctly, I think, at least on trunk. (See https://reviews.llvm.org/D42556 .) If you want to warn anyway, that's okay. Repository: rC Clang

[PATCH] D46013: [ARM] Conform to AAPCS when passing overaligned composites as arguments

efriedma added reviewers: aemerson, rjmccall. efriedma added a comment. I'm not sure Apple will want to mess with their ABI like this... adding some reviewers. Otherwise LGTM. Comment at: lib/CodeGen/TargetInfo.cpp:5790 // than ABI alignment. - uint64_t ABIAlign = 4; - u

[PATCH] D46112: Allow _Atomic to be specified on incomplete types

efriedma added a comment. If you don't want to spend too much time on C++, fine; could you add a short Objective-C test instead to make sure the trivially-copyable checks are working? What are the changes to Sema::RequireCompleteTypeImpl supposed to do? Comment at: test/Sema/

[PATCH] D47628: Detect an incompatible VLA pointer assignment

efriedma added inline comments. Comment at: lib/AST/ASTContext.cpp:8588 + Expr *E = VAT->getSizeExpr(); + if (E && VAT->getSizeExpr()->isIntegerConstantExpr(TheInt, *this)) +return std::make_pair(true, TheInt); `E && E->isIntegerCons

[PATCH] D46013: [ARM] Conform to AAPCS when passing overaligned composites as arguments

efriedma added a comment. > I'm not sure it's appropriate to impose this as an overhead on all targets. You mean the overhead of increasing the size of TypeInfo? That's fair. https://reviews.llvm.org/D46013 ___ cfe-commits mailing list cfe-commits

[PATCH] D47628: Detect an incompatible VLA pointer assignment

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D47628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

  1   2   3   4   5   6   7   8   9   10   >