[PATCH] D47829: [Driver] Accept the -fno-shrink-wrap option for GCC compatibility

efriedma added a comment. Is this something which is actually useful to control? From your description, you want to add the flag to clang not because you actually want to use it, but just because you can't figure out how to pass the right flags to your clang build. If it is useful, it should

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

efriedma added a comment. Does IR generation need any special handling for this? We add nonnull attributes in various places. Repository: rC Clang https://reviews.llvm.org/D47894 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

efriedma added a comment. The problem would come from propagating nonnull-ness from something which isn't inherently nonnull. For example, strlen has a nonnull argument, so `strlen(NULL)` is UB, therefore given `int z = strlen(x); if (x) {...}`, we can remove the null check. (Not sure we actu

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

efriedma added a comment. For the kernel, specifically, strlen() isn't an issue because it builds with -fno-builtin, but the kernel uses explicit nonnull attributes in a few places. But I guess we can assume they know what they're doing if nonnull is explicitly specified. We probably want to

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

efriedma added a comment. For potential transforms based on nonnull, see https://reviews.llvm.org/D36656, I think? Repository: rC Clang https://reviews.llvm.org/D47894 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org

[PATCH] D48040: Implement constexpr __builtin_*_overflow

efriedma added inline comments. Comment at: lib/Sema/SemaChecking.cpp:210 +Arg = S.PerformCopyInitialization(Entity, SourceLocation(), Arg); +TheCall->setArg(I, Arg.get()); } Can you split this change into a separate patch? Testcase: ``` int a() {

[PATCH] D48040: Implement constexpr __builtin_*_overflow

efriedma added inline comments. Comment at: lib/Sema/SemaChecking.cpp:210 +Arg = S.PerformCopyInitialization(Entity, SourceLocation(), Arg); +TheCall->setArg(I, Arg.get()); } erichkeane wrote: > efriedma wrote: > > Can you split this change into a sepa

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

efriedma added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:10071 + // is _MM_FROUND_CUR_DIRECTION + if (cast(Ops[4])->getZExtValue() != 4) +UsesNonDefaultRounding = true; Given we're ignoring floating-point exceptions, we should also

[PATCH] D48053: Correct behavior of __builtin_*_overflow and constexpr.

efriedma added a reviewer: rsmith. efriedma added inline comments. Comment at: lib/Sema/SemaChecking.cpp:228 + S.getASTContext(), Ty, /*consume*/ false); + Arg = S.PerformCopyInitialization(Entity, SourceLocation(), Arg); + TheCall->setArg(2, Arg.get()); --

[PATCH] D48053: Correct behavior of __builtin_*_overflow and constexpr.

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

[PATCH] D48040: Implement constexpr __builtin_*_overflow

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. I'd like to see a couple of testcases ensuring the return value is correct on overflow (we had a problem with that for __builtin_mul_overflow in the past). Otherwise LGTM. https://review

[PATCH] D48040: Implement constexpr __builtin_*_overflow

efriedma added a comment. Oh, sorry, I mixed up the two values. I meant that you should add a couple testcases to ensure the stored value is correct on overflow. https://reviews.llvm.org/D48040 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

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

efriedma added a comment. Can you give an example of what the output looks like? Please don't define multiple different classes with the same name SortClassName. It seems like you've scattered the "start" and "stop" calls all over the place; can you put the start and stop calls in the same plac

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

efriedma added a comment. Probably should have a test for something like `float x = (__uint128_t)-1;`, to make sure we print something reasonable. Comment at: lib/Sema/SemaChecking.cpp:10874 + if (Target->isSpecificBuiltinType(BuiltinType::LongDouble)) +FloatSem =

[PATCH] D53115: [COFF, ARM64] Add _ReadStatusReg and_WriteStatusReg intrinsics

efriedma added a comment. Missing Sema changes? Repository: rC Clang https://reviews.llvm.org/D53115 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53115: [COFF, ARM64] Add _ReadStatusReg and_WriteStatusReg intrinsics

efriedma added inline comments. Comment at: lib/Sema/SemaChecking.cpp:1754 + // argument here. Any constant would be converted to a register of + // the form S1_2_C3_C4_5. Let the hardware throw an exception for incorrect + // registers. This matches MSVC behavior. ---

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

efriedma added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10881-10882 +S.DiagRuntimeBehavior( +E->getExprLoc(), E, +S.PDiag(diag::warn_impcast_literal_float_to_integer) +<< E->getType() << T << PrettySourceValue << Prett

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

efriedma added a comment. Herald added a subscriber: nhaehnle. Looks like the latest update is missing a test file? https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinf

[PATCH] D53348: [AArch64] Define __ELF__ for aarch64-none-elf and other similar triples.

efriedma created this revision. efriedma added reviewers: olista01, SjoerdMeijer. Herald added a reviewer: javed.absar. Herald added a subscriber: kristof.beyls. "aarch64-none-elf" is commonly used for AArch64 baremetal toolchains. Repository: rC Clang https://reviews.llvm.org/D53348 Files:

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

efriedma added a comment. Starting multiple threads on the LLVM mailing lists with the same message is spam. Please don't do that. If you think you've run into a bug, file a bug report at bugs.llvm.org. For general questions, send an email to llvm-dev. Repository: rC Clang https://review

[PATCH] D53348: [AArch64] Define __ELF__ for aarch64-none-elf and other similar triples.

This revision was automatically updated to reflect the committed changes. Closed by commit rC344710: [AArch64] Define __ELF__ for aarch64-none-elf and other similar triples. (authored by efriedma, committed by ). Repository: rC Clang https://reviews.llvm.org/D53348 Files: lib/Basic/Targets/

[PATCH] D53115: [COFF, ARM64] Add _ReadStatusReg and_WriteStatusReg intrinsics

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

[PATCH] D53608: [builtins] Build float128 soft float builtins for x86_64.

efriedma added inline comments. Comment at: lib/builtins/fp_lib.h:111 typedef __int128_t srep_t; -typedef long double fp_t; +typedef __float128 fp_t; #define REP_C (__uint128_t) manojgupta wrote: > Changed long double to __float128 on Eli's advice in PR39376.

[PATCH] D53617: [AArch64] Support Windows stack probe command-line arguments.

efriedma created this revision. efriedma added reviewers: rnk, hans. Herald added subscribers: kristof.beyls, javed.absar. Adds support for -mno-stack-arg-probe and -mstack-probe-size. (Not really happy copy-pasting code, but that's what we do for all the other Windows targets.) Repository:

[PATCH] D53618: [AArch64] [Windows] Emit unwind tables by default.

efriedma created this revision. efriedma added a reviewer: rnk. Herald added subscribers: chrib, kristof.beyls, javed.absar. Unwind tables are necessary even in code that doesn't support exceptions. Among other things, the tables are used for setjmp(), and by debuggers. Depends on https://revie

[PATCH] D53608: [builtins] Build float128 soft float builtins for x86_64.

efriedma added a comment. `__extendxftf2` and `__trunctfxf2` are for conversions between x86 long double and `__float128`; you'll need to write implementations yourself. (That should be a separate patch.) > Thanks Eli. I also found out that GCC 4.9 does not seem to have these defined > even t

[PATCH] D53666: [Tests] Updated tests for D53342

efriedma added inline comments. Comment at: test/CodeGenOpenCL/amdgpu-nullptr.cl:513 // CHECK-LABEL: test_memset_private -// CHECK: call void @llvm.memset.p5i8.i64(i8 addrspace(5)* align 8 {{.*}}, i8 0, i64 40, i1 false) +// CHECK: call void @llvm.memset.p5i8.i64(i8 addrspace(5

[PATCH] D53608: [builtins] Build float128 soft float builtins for x86_64.

efriedma added a comment. Yes, that looks right. Repository: rCRT Compiler Runtime https://reviews.llvm.org/D53608 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53617: [AArch64] Support Windows stack probe command-line arguments.

This revision was automatically updated to reflect the committed changes. Closed by commit rC345354: [AArch64] Support Windows stack probe command-line arguments. (authored by efriedma, committed by ). Repository: rC Clang https://reviews.llvm.org/D53617 Files: lib/CodeGen/TargetInfo.cpp

[PATCH] D53514: os_log: make buffer size an integer constant expression.

efriedma added subscribers: vsk, efriedma. 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/D53514 ___ cfe-commits mailing list cfe-comm

[PATCH] D53916: [ARM64] [Windows] Implement _InterlockedExchangeAdd*_* builtins.

efriedma created this revision. efriedma added reviewers: rnk, mstorsjo. Herald added subscribers: kristina, jfb, chrib, kristof.beyls, javed.absar. These apparently need to be proper builtins to handle the Windows SDK. Repository: rC Clang https://reviews.llvm.org/D53916 Files: include/cl

[PATCH] D53916: [ARM64] [Windows] Implement _InterlockedExchangeAdd*_* builtins.

efriedma updated this revision to Diff 171852. efriedma added a comment. Get rid of unnecessary constants. Repository: rC Clang https://reviews.llvm.org/D53916 Files: include/clang/Basic/BuiltinsAArch64.def include/clang/Basic/BuiltinsARM.def lib/CodeGen/CGBuiltin.cpp lib/Headers/int

[PATCH] D53916: [ARM64] [Windows] Implement _InterlockedExchangeAdd*_* builtins.

efriedma updated this revision to Diff 171853. efriedma added a comment. Upload correct diff. Repository: rC Clang https://reviews.llvm.org/D53916 Files: include/clang/Basic/BuiltinsAArch64.def include/clang/Basic/BuiltinsARM.def lib/CodeGen/CGBuiltin.cpp lib/Headers/intrin.h test/

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in LLVM

efriedma added inline comments. Comment at: lib/Sema/SemaStmtAsm.cpp:470 +if (NS->isGCCAsmGoto() && +Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass) + break; jyu2 wrote: > efriedma wrote: > > This looks suspicious; an AddrLabelE

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in LLVM

efriedma added inline comments. Comment at: lib/Analysis/CFG.cpp:1474 + appendScopeBegin(JT.block, VD, G); + addSuccessor(B, JT.block); +} Please don't copy-paste code. Comment at: lib/Sema/SemaStmtAsm.cpp:470 +if

[PATCH] D56620: [COFF, ARM64] Include stdlib.h in arm64intr.h

efriedma added a comment. Have you verified this matches MSVC? (IIRC the only reason we include stdlib.h on x86 is so we can define _mm_malloc.) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56620/new/ https://reviews.llvm.org/D56620

[PATCH] D56620: [COFF, ARM64] Declare intrinsics: __nop, _byteswap_[ushort/ulong/uint64]

efriedma added inline comments. Comment at: lib/Headers/intrin.h:569 +unsigned __int64 _byteswap_uint64 (unsigned __int64 val); +void __nop(); #endif Isn't there already a declaration of __nop in intrin.h? (Line 100.) CHANGES SINCE LAST ACTION https://revie

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in LLVM

efriedma added inline comments. Comment at: lib/Sema/SemaStmtAsm.cpp:470 +if (NS->isGCCAsmGoto() && +Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass) + break; jyu2 wrote: > jyu2 wrote: > > efriedma wrote: > > > jyu2 wrote: > > >

[PATCH] D56671: [COFF, ARM64] Add __nop intrinsic

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM I'm assuming this isn't on the list of intrinsics that we must not implement as an inline function in intrin.h... if it is, we'll have to reimplement it at some point. But this is c

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

efriedma added inline comments. Comment at: lib/Sema/SemaStmtAsm.cpp:470 +if (NS->isGCCAsmGoto() && +Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass) + break; jyu2 wrote: > efriedma wrote: > > jyu2 wrote: > > > jyu2 wrote: > > >

[PATCH] D56685: [COFF, ARM64] Declare __byteswap intrinsics

efriedma added inline comments. Comment at: lib/Headers/arm64intr.h:50 +#define _byteswap_ulong __builtin_bswap32 +#define _byteswap_uint64 __builtin_bswap64 + These should be available for all Microsoft targets, I think. Please define these as inline functions

[PATCH] D56685: [COFF, ARM64] Add __byteswap intrinsics

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Comment at: test/Headers/ms-arm64-intrin.cpp:19 __nop(); } + While you're here, please fix all four tests to just check the IR. CHANGES SINCE

[PATCH] D56685: [COFF, ARM64] Add __byteswap intrinsics

efriedma added a comment. Before you commit, please verify this actually compiles if you include both this intrin.h and stdlib.h from the Microsoft SDK, in either order. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56685/new/ https://reviews.llvm.org/D56685

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

efriedma added inline comments. Comment at: include/clang/Basic/DiagnosticASTKinds.td:214 + def err_asm_invalid_operand_number_for_goto_labels : Error < +"invalid operand number which isn't point to a goto label in asm string">; } Grammar. Also, "invalid o

[PATCH] D56748: [EH] Rename llvm.x86.seh.recoverfp intrinsic to llvm.eh.recoverfp

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56748/new/ https://reviews.llvm.org/D56748 ___ cfe-commit

[PATCH] D56748: [EH] Rename llvm.x86.seh.recoverfp intrinsic to llvm.eh.recoverfp

This revision was automatically updated to reflect the committed changes. Closed by commit rL351284: [EH] Rename llvm.x86.seh.recoverfp intrinsic to llvm.eh.recoverfp (authored by efriedma, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.or

[PATCH] D56821: [CodeGen] Always use string computed in Sema for PredefinedExpr

efriedma created this revision. efriedma added reviewers: rjmccall, rsmith. We can't use any other string, anyway, because its type wouldn't match the type of the PredefinedExpr. With this change, we don't compute a "nice" name for the __func__ global when it's used in the initializer for a con

[PATCH] D56852: [AArch64] Use LLU for 64-bit crc32 arguments

efriedma added a comment. While you're here, can you also fix `__builtin_arm_rbit64`, `__builtin_arm_rsr64`, and `__builtin_arm_wsr64`? Comment at: include/clang/Basic/BuiltinsAArch64.def:54 +BUILTIN(__builtin_arm_crc32d, "UiUiLLUi", "nc") +BUILTIN(__builtin_arm_crc32cd, "UiUi

[PATCH] D56852: [AArch64] Use LL for 64-bit arguments

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Comment at: test/CodeGen/builtins-arm64.c:2 // RUN: %clang_cc1 -triple arm64-unknown-linux -disable-O0-optnone -emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s

[PATCH] D56821: [CodeGen] Always use string computed in Sema for PredefinedExpr

This revision was automatically updated to reflect the committed changes. Closed by commit rL351766: [CodeGen] Always use string computed in Sema for PredefinedExpr (authored by efriedma, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/

[PATCH] D57135: [ExprConstant] Unify handling of array init with lvalues.

efriedma created this revision. efriedma added a reviewer: rsmith. This should have no functional effect; evaluation is still generating the same result. But the assertions and comments should make it more clear what's actually happening. Part of a fix to https://bugs.llvm.org/show_bug.cgi?id

[PATCH] D57209: Revert "[AArch64] Use LL for 64-bit intrinsic arguments"

efriedma added a comment. Oh, rsr and wsr are macros, not functions... I was confused how it would break. The correct signature for __builtin_arm_rsr64 is "WUicC*", I think. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57209/new/ https://reviews.llvm.org/D57209

[PATCH] D57210: [AArch64] Make the test for rsr and rsr64 stricter

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57210/new/ https://reviews.llvm.org/D57210 ___ cfe-commit

[PATCH] D57135: [ExprConstant] Unify handling of array init with lvalues.

efriedma abandoned this revision. efriedma added a comment. I'm now convinced this is the wrong approach. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57135/new/ https://reviews.llvm.org/D57135 ___ cfe-commits maili

[PATCH] D56852: [AArch64] Use LL for 64-bit arguments

efriedma accepted this revision. efriedma added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56852/new/ https://reviews.llvm.org/D56852 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/

[PATCH] D53919: [X86] Don't allow illegal vector types to return by direct value on x86-64.

efriedma added a comment. As far as I know, according to the ABI docs, it's impossible to return a 256-bit vector from a function if AVX is disabled. Given that we aren't rejecting the testcase, we're effectively implementing a new "no-AVX" ABI variant. That variant is not defined anywhere, so

[PATCH] D53919: [X86] Don't allow illegal vector types to return by direct value on x86-64.

efriedma added a comment. Whatever you call it, the question remains; what specification and/or compiler are we trying to be compatible with? Repository: rC Clang https://reviews.llvm.org/D53919 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D53919: [X86] Don't allow illegal vector types to return by direct value on x86-64.

efriedma added a comment. The ABI document before AVX was introduced didn't say anything at all about 256-bit vectors. So we're talking about compatibility with some other compiler. Which compiler? Repository: rC Clang https://reviews.llvm.org/D53919 ___

[PATCH] D53919: [X86] Don't allow illegal vector types to return by direct value on x86-64.

efriedma added a comment. I just tried clang 3.3, and as far as I can tell it behaves the same way as trunk. If the argument is that we should be compatible with gcc on Linux, that's fine, but we should explicitly state that in a comment. Repository: rC Clang https://reviews.llvm.org/D5391

[PATCH] D53541: [COFF, ARM64] Support SEH for ARM64 Windows

efriedma added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:471 + } + EH |= SEHExceptions; + I'm suspicious about adding a new default for AArch64 specifically; why do we need this if x86-64 doesn't? https://reviews.llvm.org/D53541 ___

[PATCH] D53919: [X86] Don't allow illegal vector types to return by direct value on x86-64.

efriedma added a comment. No, AVX512 wasn't even announced when clang 3.3 came out. Repository: rC Clang https://reviews.llvm.org/D53919 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co

[PATCH] D53916: [ARM64] [Windows] Implement _InterlockedExchangeAdd*_* builtins.

This revision was automatically updated to reflect the committed changes. Closed by commit rC345779: [ARM64] [Windows] Implement _InterlockedExchangeAdd*_* builtins. (authored by efriedma, committed by ). Repository: rC Clang https://reviews.llvm.org/D53916 Files: include/clang/Basic/Builti

[PATCH] D53618: [AArch64] [Windows] Emit unwind tables by default.

This revision was automatically updated to reflect the committed changes. Closed by commit rC345781: [AArch64] [Windows] Emit unwind tables by default. (authored by efriedma, committed by ). Changed prior to commit: https://reviews.llvm.org/D53618?vs=170767&id=172024#toc Repository: rC Clang

[PATCH] D53919: [X86] Don't allow illegal vector types to return by direct value on x86-64.

efriedma added a comment. With both 3.3 and trunk (I don't have a 7.0 handy; I can build it if it would be helpful): movaps .LCPI0_0(%rip), %xmm0 # xmm0 = [48,49,50,51] movaps .LCPI0_1(%rip), %xmm1 # xmm1 = [52,53,54,55] movaps .LCPI0_2(%rip), %xmm2 # xmm2 = [56,57,97,98] movaps

[PATCH] D53960: [COFF, ARM64] Implement llvm.addressofreturnaddress intrinsic

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/D53960 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D53514: os_log: make buffer size an integer constant expression.

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

[PATCH] D53998: [COFF, ARM64] Change setjmp for AArch64 Windows to use Intrinsic.sponentry

efriedma added a comment. This is exactly the same as https://reviews.llvm.org/D53684, right? You don't need another review. Repository: rC Clang https://reviews.llvm.org/D53998 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lis

[PATCH] D54046: [COFF, ARM64] Implement InterlockedExchange*_* builtins

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/D54046 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D54062: [COFF, ARM64] Implement InterlockedCompareExchange*_* builtins

efriedma added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:233 +static Value *EmitAtomicCmpXchgValue(CodeGenFunction &CGF, const CallExpr *E, +AtomicOrdering SuccessOrdering = AtomicOrdering::SequentiallyConsistent) { Please rename this function;

[PATCH] D54062: [COFF, ARM64] Implement InterlockedCompareExchange*_* builtins

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

[PATCH] D54068: [COFF, ARM64] Implement InterlockedDecrement*_* builtins

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/D54068 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D54067: [COFF, ARM64] Implement InterlockedIncrement*_* builtins

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/D54067 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D54066: [COFF, ARM64] Implement InterlockedAnd*_* builtins

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/D54066 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D54065: [COFF, ARM64] Implement InterlockedXor*_* builtins

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/D54065 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D54063: [COFF, ARM64] Implement InterlockedOr*_* builtins

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/D54063 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

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

efriedma added a comment. > It seems you were well aware that you are changing the semantics of FP > operation here by ignoring the signaling/quiet portion of the immediate. There's ongoing work to support code that accesses the floating point environment (see http://llvm.org/docs/LangRef.html

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

efriedma added a comment. Yes, in constrained-fp mode we might need intrinsics, at least short-term. I assume you'll probably add target-independent constrained masked fp vector operations at some point, but that's probably not a priority. But that still leaves two problems. One, clang doesn

[PATCH] D55021: Mark __builtin_shufflevector as using custom type checking

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55021/new/ https://reviews.llvm.org/D55021 ___ cfe-commits mailing list cfe-commi

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

efriedma added a comment. I'm not sure that putting a warning that can be disabled really helps here; anyone who needs the option will just disable the warning anyway, and then users adding additional options somewhere else in the build system will miss the warning. Instead, it would probably

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

efriedma added a comment. Or actually, if you really want to discourage people from using them, maybe we could use the LLVM version number in the name, like "--unstable-llvm-option-8" (which would change to "--unstable-llvm-option-9" in in 9.0, etc.). This would allow developers to continue us

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

efriedma added a comment. > In fact -mllvm is used extensively in a lot of lit/FileCheck tests, so that's > also going to cause problems. I count 13 uses of -mllvm in driver commands in-tree, and some of those are actually testing the flag itself. That doesn't seem like a lot. (This is exclu

[PATCH] D57590: [ASTImporter] Improve import of FileID.

efriedma added a comment. I don't know anything about this particular patch, but you aren't allowed to set deadlines like that; the patch review process requires that the patch is actually reviewed. If you aren't getting a response, ask on cfe-dev. Repository: rC Clang CHANGES SINCE LAST A

[PATCH] D58375: [Clang][NewPM] Disable tests that are broken under new PM

efriedma added a comment. I thought the plan was to fix whatever issue was causing the -O0 tests to fail, then rebase on top of that? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58375/new/ https://reviews.llvm.org/D58375

[PATCH] D57335: [IR] Don't assume all functions are 4 byte aligned

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. Probably worth sending an email to llvmdev noting that the datalayout is changing before you merge this, so a wider audience can review the IR aspects of the change. Otherwise LGTM Repo

[PATCH] D58649: Fix inline assembler constraint validation

efriedma added inline comments. Comment at: include/clang/Basic/TargetInfo.h:860 + if (!ImmSet.empty()) +return ImmSet.count(Value.getZExtValue()) != 0; + return !ImmRange.isConstrained || (Value.sge(ImmRange.Min) && Value.sle(ImmRange.Max)); I

[PATCH] D58236: Make address space conversions a bit stricter.

efriedma added a comment. Generally, with an explicit cast, C allows any pointer cast with a reasonable interpretation, even if the underlying operation is suspicious. For example, you can cast an "long*" to a "int*" (as in "(int*)(long*)p") without any complaint, even though dereferencing the

[PATCH] D58236: Make address space conversions a bit stricter.

efriedma added a comment. > I think trying to reject code that is doing something dangerous is a good > thing! Refusing to compile code which is suspicious, but not forbidden by the specification, will likely cause compatibility issues; there are legitimate reasons to use casts which look weir

[PATCH] D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890)

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGStmt.cpp:1852 +IntResult = +llvm::APSInt::get(EVResult.Val.getLValueOffset().getQuantity()); + else This always returns an APSInt with width 64; is that really right? I gues

[PATCH] D58236: Make address space conversions a bit stricter.

efriedma added a comment. In D58236#1416765 , @Anastasia wrote: > In D58236#1414069 , @efriedma wrote: > > > > I think trying to reject code that is doing something dangerous is a good > > > thing! > > > > Refusing

[PATCH] D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890)

efriedma added a comment. > This fails with the new asm constraint checks, since the dead branch is never > pruned. As far as I know, we didn't touch "i", only "n". Is there a bug filed for the issue you're describing? Comment at: clang/lib/CodeGen/CGStmt.cpp:1852 +

[PATCH] D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890)

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58821/new/ https://reviews.llvm.org/D58821 ___ cfe-commits mailing list cfe-commi

[PATCH] D58811: [clang] Fix misuses of char width to char align

efriedma added a comment. > The patch have been tested for a proprietary architecture, where there is a > difference between align and width of char. The C standard requires that both `alignof(char)` and `sizeof(char)` must equal 1, and therefore must equal each other. Could you clarify the

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. I'm fine merging in this state... but please wait for rsmith to respond before you merge. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571

[PATCH] D59094: [ARM] Fix bug 39982 - pcs("aapcs-vfp") is not consistent

efriedma added a comment. It looks like every call to getABIKind() is doing the wrong thing; I'm worried that's going to lead to some sort of subtle miscompile without some sort of centralized fix to compute the correct calling convention for every place that checks it. Not sure how to write a

[PATCH] D56990: Bugfix for Replacement of tied operand of inline asm

efriedma added a comment. We expect that tests for clang IR generation should look something like clang/test/CodeGen/asm-inout.c. Even though a test like that isn't directly testing the overall behavior, we try to separate tests of clang's behavior from tests of LLVM's behavior. It gives bett

[PATCH] D56990: Bugfix for Replacement of tied operand of inline asm

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM; I'll merge it tonight or tomorrow. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56990/new/ https://reviews.llvm.org/D56990 __

[PATCH] D59094: [ARM] Fix bug 39982 - pcs("aapcs-vfp") is not consistent

efriedma added a comment. Looking more carefully, I guess none of the other places actually distinguish between ARMABIInfo::AAPCS_VFP and ARMABIInfo::AAPCS, so I guess the current change is sufficient given the calling-convention attributes that are likely to be used in practice. Please pull t

[PATCH] D56990: Bugfix for Replacement of tied operand of inline asm

This revision was automatically updated to reflect the committed changes. Closed by commit rC356196: [CodeGen] Consider tied operands when adjusting inline asm operands. (authored by efriedma, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56990/new/

[PATCH] D59094: [ARM] Fix bug 39982 - pcs("aapcs-vfp") is not consistent

efriedma accepted this revision. efriedma added a reviewer: t.p.northover. efriedma added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/CodeGen/TargetInfo.cpp:6023 + // Variadic functions should always marshal to the base standard. + b

[PATCH] D57335: [IR] Don't assume all functions are 4 byte aligned

efriedma added a comment. Anyone looking at updating the datalayout for other platforms? (If nobody is looking at all, I'll try to find some time next week, I guess.) Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57335/new/ https://reviews.llvm.org/D57335 ___

<    1   2   3   4   5   6   7   8   9   10   >