[PATCH] D123950: Look through calls to std::addressof to compute pointer alignment.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGecc8479a01d3: Look through calls to std::addressof to compute pointer alignment. (authored by efriedma). Repository: rG LLVM Github Monorepo CHAN

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

efriedma added a comment. (Please mark this "closed" to reflect the fact that it was merged.) Comment at: clang/lib/Sema/JumpDiagnostics.cpp:935 +LabelStmt *Label = cast(To); +Label->setSideEntry(true); } tentzen wrote: > rjmccall wrote: > > This doe

[PATCH] D116088: [compiler-rt] Implement ARM atomic operations for architectures without SMP support

efriedma added a comment. I'm concerned providing these is going to cause issues. The provided implementation are not atomic. Blindly assuming that the user is compiling for a target that doesn't have pre-emptible threads seems like a bad idea. How do you expect users to use these methods, an

[PATCH] D116088: [compiler-rt] Implement ARM atomic operations for architectures without SMP support

efriedma added a comment. Posted D120026 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116088/new/ https://reviews.llvm.org/D116088 ___ cfe-commits mailing list cfe-c

[PATCH] D116088: [compiler-rt] Implement ARM atomic operations for architectures without SMP support

efriedma reopened this revision. efriedma added a comment. This revision is now accepted and ready to land. Reverted in 0389f2edf7 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D124026: [Headers][MSVC] Define wchar_t in stddef.h like MSVC if not using the builtin type

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124026/new/ https://reviews.llvm.org/D124026 ___

[PATCH] D124158: [Clang][Attr] Skip adding noundef attribute to arguments when function has convergent attribute

efriedma added a comment. The issue you're describing sounds like it's specific to @__shfl_sync. In general, in C++, you aren't allowed to read from an uninitialized variable; see [basic.indet] in the standard. But if your testcase doesn't have undefined behavior, CUDA language rules must som

[PATCH] D124158: [Clang][Attr] Skip adding noundef attribute to arguments when function has convergent attribute

efriedma added a comment. In D124158#3480384 , @arsenm wrote: > I'm thinking noundef is a bit of red herring here. The real problem seems to > be arising from the assume call which is inserted, which now introduces the > assumption that the lane ID must

[PATCH] D124642: [WIP] Add support for return from an SEH __finally block.

efriedma created this revision. efriedma added reviewers: asmith, tentzen, rnk, mstorsjo, JosephTremoulet, dpaoliello, rjmccall. Herald added subscribers: pengfei, hiraditya. Herald added a project: All. efriedma requested review of this revision. Herald added a subscriber: jdoerfert. Herald added

[PATCH] D124642: [WIP] Add support for return from an SEH __finally block.

efriedma updated this revision to Diff 426486. efriedma edited the summary of this revision. efriedma added a comment. Figured out the __finally nesting thing; we were missing unwind table entries. (This is a consequence of the fact that our unwind tables don't cover the entire function, just i

[PATCH] D124642: [WIP] Add support for return from an SEH __finally block.

efriedma added a comment. I'm currently thinking it makes sense to introduce an intrinsic "void @llvm.localunwind() noreturn". No arguments. Only valid with SEH personality; must unwind to a catchpad with a filter named __IsLocalUnwind, or something like that. It would basically just lower t

[PATCH] D124642: [WIP] Add support for return from an SEH __finally block.

efriedma updated this revision to Diff 426814. efriedma edited the summary of this revision. efriedma added a comment. Switch to use llvm.seh_localunwind intrinsic. I'd appreciate any feedback at this point. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D124998: [AArch64][SVE] Add aarch64_sve_pcs attribute to Clang

efriedma added a comment. If you're really concerned about the size of FunctionProtoType increasing, can we just shove the infrequently used calling convention bits into TrailingObjects? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124998/new/ h

[PATCH] D124998: [AArch64][SVE] Add aarch64_sve_pcs attribute to Clang

efriedma added a comment. In D124998#3494426 , @erichkeane wrote: > In D124998#3494424 , @efriedma > wrote: > >> If you're really concerned about the size of FunctionProtoType increasing, >> can we just shove t

[PATCH] D124998: [AArch64][SVE] Add aarch64_sve_pcs attribute to Clang

efriedma added a comment. I don't want to change with the distinction between FunctionProtoType and FunctionNoProtoType; that would involve auditing a bunch of code we don't really want to mess with. FunctionProtoType and FunctionNoProtoType don't have any member variables at the moment, thoug

[PATCH] D125094: [ARM][Thumb] Command-line option to ensure AAPCS compliant Frame Records

efriedma added a comment. We probably want testcases for accessing various parts of the stack frame with FP stored in r11; for example, can we correctly access arguments passed on the stack if there's stack realignment? Comment at: llvm/lib/Target/ARM/Thumb1FrameLowering.cpp:

[PATCH] D125177: Recognize scope of thread local variables in CFGBuilder

efriedma added a comment. Can we just use VarDecl::hasLocalStorage()? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125177/new/ https://reviews.llvm.org/D125177 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D125177: Recognize scope of thread local variables in CFGBuilder

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125177/new/ https://reviews.llvm.org/D125177 ___

[PATCH] D129476: [AArch64][SVE] Prefer SIMD&FP variant of clast[ab]

efriedma added a comment. Are you saying that it's faster to use clasta targeting a float register, then move the result to an integer register, rather than use the integer form directly? Or is the issue just that we want to split the operations in case we can simplify the resulting bitcast?

[PATCH] D129476: [AArch64][SVE] Prefer SIMD&FP variant of clast[ab]

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. Please update the comment, then LGTM Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:801 + Intrins

[PATCH] D129802: [DRAFT] Implementing new atomic orderings in LLVM and generate barriers for legacy __sync builtins. Support corresponding memory model in outline atomics as well.

efriedma added a comment. `atomicrmw add [...] sync_seq_cst` is supposed to be equivalent to `fence seq_cst; atomicrmw add seq_cst; fence seq_cst`, I think. The extra fences aren't necessary for most uses of `__sync_*`, but the difference is theoretically visible for exotic synchronization prim

[PATCH] D129881: [C] Strengthen -Wint-conversion to default to an error

efriedma added a comment. Given this is an on-by-default warning already, I don't expect changing it to an error by default to have much impact in practice. Changing it fine. Comment at: clang/lib/Sema/SemaExprCXX.cpp:8483 +if ((SemaRef.getLangOpts().CPlusPlus && Trap.has

[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

efriedma added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5128 +// to the function itself; it points to a stub for the compiler. +// FIXME: We also need to emit an entry thunk. +SmallString<256> MangledName; bcl5980 wrote: > A he

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

efriedma accepted this revision. efriedma added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D28213/new/ https://reviews.llvm.org/D28213 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D129881: [C] Strengthen -Wint-conversion to default to an error

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

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

efriedma added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:4267 + bool IsConstant = CLETy.isConstant(Info.Ctx); + if (!IsConstant && CLETy->isArrayType()) { +Info.FFDiag(Conv); Is the "isArrayType()" check here necessary? CHANG

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

efriedma added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:4267 + bool IsConstant = CLETy.isConstant(Info.Ctx); + if (!IsConstant && CLETy->isArrayType()) { +Info.FFDiag(Conv); serge-sans-paille wrote: > efriedma wrote: > > Is th

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

efriedma added a comment. I think you're looking at old documentation? Here's what the current page (https://gcc.gnu.org/onlinedocs/gcc/Compound-Literals.html) has to say: In C, a compound literal designates an unnamed object with static or automatic storage duration. In C++, a compound lite

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

efriedma added a comment. I think so, yes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124038/new/ https://reviews.llvm.org/D124038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

efriedma created this revision. Herald added subscribers: zzheng, kristof.beyls. Herald added a project: All. efriedma requested review of this revision. Herald added a project: clang. Part of initial Arm64EC patchset. For the Arm64EC ABI, ARM64 functions have an alternate name. For C code, this

[PATCH] D125419: [Arm64EC 7/?] clang side of Arm64EC varargs ABI.

efriedma created this revision. Herald added subscribers: zzheng, kristof.beyls. Herald added a project: All. efriedma requested review of this revision. Herald added a project: clang. Part of initial Arm64EC patchset. Don't try to duplicate the existing logic; instead, just call the actual code

[PATCH] D124701: [clang] Honor __attribute__((no_builtin("foo"))) on functions

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:5219 +// function. +else if (!CGF.CurFn->getAttributes().hasFnAttr(AttributeNoBuiltin)) return CGCallee::forBuiltin(builtinID, FD); steplong wrote: > hans wrote: > > What if C

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

efriedma added a comment. Updated messed up formatting? Otherwise, I guess this is fine. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124038/new/ https://reviews.llvm.org/D124038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

efriedma added inline comments. Comment at: clang/test/Sema/warn-strict-prototypes.m:20 + // know it's a block when diagnosing. + void (^block2)(void) = ^void() { // expected-warning {{a function declaration without a prototype is deprecated in all versions of C}} }; --

[PATCH] D125094: [ARM][Thumb] Command-line option to ensure AAPCS compliant Frame Records

efriedma added inline comments. Comment at: llvm/test/CodeGen/Thumb/frame-access.ll:41 +; CHECK-ATPCS: ldrr0, [sp, #32] +; CHECK-AAPCS: ldrr0, [r11, #8] ; CHECK-NEXT: strr3, [sp] `ldr r0, [r11, #8]` is not a valid Thumb1 instruction. Repository:

[PATCH] D125521: [Diagnostic] Warn if the size argument of memset is character literal zero

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125521/new/ https://reviews.llvm.org/D125521 ___

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

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/D124038/new/ https://reviews.llvm.org/D124038 ___ cfe-commits mailing list cfe-commi

[PATCH] D125557: [APInt] Remove all uses of zextOrSelf, sextOrSelf and truncOrSelf

efriedma added subscribers: reames, efriedma. efriedma added inline comments. Comment at: llvm/lib/IR/ConstantRange.cpp:724 auto BW = getBitWidth(); -APInt Min = APInt::getMinValue(BW).zextOrSelf(ResultBitWidth); -APInt Max = APInt::getMaxValue(BW).zextOrSelf(ResultB

[PATCH] D125557: [APInt] Remove all uses of zextOrSelf, sextOrSelf and truncOrSelf

efriedma added inline comments. Comment at: llvm/lib/IR/ConstantRange.cpp:724 auto BW = getBitWidth(); -APInt Min = APInt::getMinValue(BW).zextOrSelf(ResultBitWidth); -APInt Max = APInt::getMaxValue(BW).zextOrSelf(ResultBitWidth); +APInt Min = APInt::getMinValue(

[PATCH] D91157: [AArch64] Out-of-line atomics (-moutline-atomics) implementation.

efriedma added a comment. https://github.com/llvm/llvm-project/issues/29472 was never fixed; whatever issues exist with -moutline-atomics also exist with -mno-outline-atomics. (I don't think anyone has run into any practical issues with this, so it hasn't been a priority for anyone.) Reposit

[PATCH] D123649: Allow flexible array initialization in C++.

efriedma added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4639 + getDataLayout().getTypeAllocSize(Init->getType())); + assert(VarSize == CstSize && "Emitted constant has unexpected size"); +#endif ahatanak wrote: > This asserti

[PATCH] D125557: [APInt] Remove all uses of zextOrSelf, sextOrSelf and truncOrSelf

efriedma added inline comments. Comment at: llvm/lib/IR/ConstantRange.cpp:724 auto BW = getBitWidth(); -APInt Min = APInt::getMinValue(BW).zextOrSelf(ResultBitWidth); -APInt Max = APInt::getMaxValue(BW).zextOrSelf(ResultBitWidth); +APInt Min = APInt::getMinValue(

[PATCH] D116088: [compiler-rt] Implement ARM atomic operations for architectures without SMP support

efriedma added a comment. On a target that doesn't support SMP, you don't need memory barriers, sure. (I think we'd want a CMake flag to explicitly assert that you're only going to run the code on chips without SMP.) That doesn't really solve your issue, though. To implement atomic compare-a

[PATCH] D125094: [ARM][Thumb] Command-line option to ensure AAPCS compliant Frame Records

efriedma added inline comments. Comment at: llvm/lib/Target/ARM/ARMFrameLowering.cpp:1844 +for (const auto &MI : MBB) + if (MI.getOpcode() == ARM::tSTRspi || MI.getOpcode() == ARM::tSTRi) +for (const auto &Op : MI.operands()) How do you end up wi

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable (1/3)

efriedma added a comment. I don't really understand how this is supposed to interact with D125292 ; even if you strip the readnone attribute from the call instruction, we'll still treat a call to the intrinsic as readnone. I think I'd prefer to lower the intri

[PATCH] D123649: Allow flexible array initialization in C++.

efriedma added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4639 + getDataLayout().getTypeAllocSize(Init->getType())); + assert(VarSize == CstSize && "Emitted constant has unexpected size"); +#endif ahatanak wrote: > efriedma wro

[PATCH] D126023: [MSVC, ARM64] Add __writex18 intrinsics

efriedma added a comment. `uxtw #3` makes me think you're not generating the right code... the zero-extension hopefully doesn't matter, but the shift is significant. Maybe should be generating `getelementptr i8`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D125094: [ARM][Thumb] Command-line option to ensure AAPCS compliant Frame Records

efriedma added inline comments. Comment at: llvm/lib/Target/ARM/ARMFrameLowering.cpp:1863 // to combine multiple loads / stores. - bool CanEliminateFrame = true; + bool CanEliminateFrame = !requiresAAPCSFrameRecord(MF); bool CS1Spilled = false; Should thi

[PATCH] D126023: [MSVC, ARM64] Add __writex18 intrinsics

efriedma added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:9977 +StoreInst *Store = Builder.CreateAlignedStore( +Val, Ptr, getContext().getTypeAlignInChars(E->getType())); +return Store; I think I'd prefer just "CharUnits::One()",

[PATCH] D126023: [MSVC, ARM64] Add __writex18 intrinsics

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126023/new/ https://reviews.llvm.org/D126023 ___

[PATCH] D126024: [MSVC, ARM64] Add __readx18 intrinsics

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126024/new/ https://reviews.llvm.org/D126024 ___

[PATCH] D116088: [compiler-rt] Implement ARM atomic operations for architectures without SMP support

efriedma added a comment. In D116088#3534100 , @kpdev42 wrote: > 1. Sets memory barrier > 2. Calls atomic load from memory location > 3. Modifies data > 4. Calls atomic store to memory location > 5. Checks that operation is consistent, if not goes to step

[PATCH] D66122: [CodeGen] Emit dynamic initializers for static TLS vars in outlined scopes

efriedma added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:479 +return a_deps.find(b) != a_deps.end() +|| b->getLocation() < a->getLocation(); // ensure deterministic ordering + }); Is the call to a_deps.find() he

[PATCH] D66122: [CodeGen] Emit dynamic initializers for static TLS vars in outlined scopes

efriedma added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:479 +return a_deps.find(b) != a_deps.end() +|| b->getLocation() < a->getLocation(); // ensure deterministic ordering + }); Prince781 wrote: > efriedma wro

[PATCH] D66122: [CodeGen] Emit dynamic initializers for static TLS vars in outlined scopes

efriedma added a comment. Added a few more minor comments. It looks like the consensus on std-discussion is actually that your testcase has undefined behavior? That seems like an awful conclusion, and the standard text doesn't really seem to support it. (I mean, I guess you could argue that "

[PATCH] D66122: [CodeGen] Emit dynamic initializers for static TLS vars in outlined scopes

efriedma added a comment. Oh, I somehow forgot that was legal. :( That breaks this whole approach (well, maybe the lambda could capture "w", but that seems way too complicated). So we're left with a few possibilities: 1. an error 2. undefined behavior (probably we would want to warn in this c

[PATCH] D66839: Fix stack address builtin for negative numbers

efriedma added a comment. We usually prefer to generate error messages for incorrect parameters to builtins in SemaChecking.cpp. I don't think there's really an infinite loop here. The parameter to __builtin_frame_address is an unsigned integer; values greater than 0x7FFU aren't special.

[PATCH] D66862: Make lround builtin constexpr (and others)

efriedma added a comment. It's probably worth adding testcases for 0.5 and -0.5. I think the current implementation behaves correctly, but it would be easy to mess up with a small change to the code. Are you intentionally excluding `__builtin_lroundl`/`__builtin_llroundl`?

[PATCH] D66839: Fix stack address builtin for negative numbers

efriedma added a comment. In the context of __builtin_frame_address, an arbitrary limit is probably okay. Maybe something like 0x, which is larger than anyone would realistically use, but doesn't take a crazy amount of time to compile. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D67028: Use musttail for variadic method thunks when possible

efriedma added a comment. Do we have test coverage for a variadic, covariant thunk for a function without a definition? I don't think there's any way for us to actually emit that, but we should make sure the error message is right. I'm a little concerned that using musttail thunks with the Ita

[PATCH] D67028: Use musttail for variadic method thunks when possible

efriedma added a comment. Oops, meant to actually include the testcase in my last comment: struct V1 { }; struct V2 : virtual V1 { }; struct A { virtual V1 *f(int,...); }; struct B : A { virtual void b(); virtual V2 *f(int,...); }; B b; Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D67028: Use musttail for variadic method thunks when possible

efriedma added a comment. > In the MS ABI, deriving a new class may require the creation of new thunks > for methods that were not overridden, so we can't use the same trick. Yes. MSVC emits an error message "covariant returns with multiple or virtual inheritance not supported for varargs func

[PATCH] D67028: Use musttail for variadic method thunks when possible

efriedma added a comment. > In your test case, we hit the early return that I linked to, so we don't try > to clone, and we don't need to emit an error. Yes, that's what happens with the Itanium ABI; what happens with the MS ABI? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D67028: Use musttail for variadic method thunks when possible

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67028/new/ https://reviews.llvm.org/D67028 ___

[PATCH] D34801: [coverage] Make smaller regions for the first case of a switch.

efriedma added a comment. I'm going to look over the overall algorithm one more time to make sure this direction makes sense. The current code for ending regions on break/continue/return/noreturn doesn't really work well, and the way we handle multiple consecutive case statments isn't really r

[PATCH] D34801: [coverage] Make smaller regions for the first case of a switch.

This revision was automatically updated to reflect the committed changes. Closed by commit rL309901: [coverage] Make smaller regions for the first case of a switch. (authored by efriedma). Changed prior to commit: https://reviews.llvm.org/D34801?vs=104581&id=109456#toc Repository: rL LLVM h

[PATCH] D36250: [coverage] Special-case calls to noreturn functions.

efriedma created this revision. The code after a noreturn call doesn't execute. The pattern in the testcase is pretty common in LLVM (a switch with a default case that calls llvm_unreachable). Repository: rL LLVM https://reviews.llvm.org/D36250 Files: lib/CodeGen/CoverageMappingGen.cpp

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

efriedma added a comment. Do you actually need a new flag for this? "-fno-common" will ensure clang doesn't generate globals with common linkage. Repository: rL LLVM https://reviews.llvm.org/D35917 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

efriedma added inline comments. Comment at: cfe/trunk/lib/CodeGen/TargetInfo.cpp: + GVar->setLinkage(llvm::GlobalValue::ExternalLinkage); + GVar->setSection("rodata"); +} Also, this change is clearly unacceptable; among other things,

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

efriedma added a comment. LLVM never puts constant data into the BSS. See isSuitableForBSS in lib/Target/TargetLoweringObjectFile.cpp. (gcc's behavior is just weird... apparently, whether or not constant data is placed in the BSS with -fno-common depends on the syntactic form of the initializ

[PATCH] D36250: [coverage] Special-case calls to noreturn functions.

This revision was automatically updated to reflect the committed changes. Closed by commit rL309995: [coverage] Special-case calls to noreturn functions. (authored by efriedma). Changed prior to commit: https://reviews.llvm.org/D36250?vs=109457&id=109644#toc Repository: rL LLVM https://revi

[PATCH] D36250: [coverage] Special-case calls to noreturn functions.

efriedma reopened this revision. efriedma added a comment. This revision is now accepted and ready to land. This was reverted. Failing function, from include/clang/AST/Type.h: inline bool Type::isImageType() const { #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) is##Id##Type()

[PATCH] D36250: [coverage] Special-case calls to noreturn functions.

efriedma updated this revision to Diff 109838. efriedma added a comment. Add missing call to handleFileExit(). Not sure I'm really calling it in the right place, but this seems to work. Repository: rL LLVM https://reviews.llvm.org/D36250 Files: lib/CodeGen/CoverageMappingGen.cpp test/Co

[PATCH] D36250: [coverage] Special-case calls to noreturn functions.

efriedma updated this revision to Diff 110249. efriedma added a comment. Update to call VisitStmt(E) Repository: rL LLVM https://reviews.llvm.org/D36250 Files: lib/CodeGen/CoverageMappingGen.cpp test/CoverageMapping/md.cpp test/CoverageMapping/switch.cpp Index: test/CoverageMapping/s

[PATCH] D36250: [coverage] Special-case calls to noreturn functions.

This revision was automatically updated to reflect the committed changes. Closed by commit rL310406: [coverage] Special-case calls to noreturn functions. (authored by efriedma). Changed prior to commit: https://reviews.llvm.org/D36250?vs=110249&id=110267#toc Repository: rL LLVM https://revi

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

efriedma added a comment. Please propose a patch for LangRef clarifying what exactly it means to have a section attribute on a declaration. Comment at: test/CodeGenCXX/extern-section-attribute.cpp:1 +// RUN: %clang_cc1 -emit-llvm %s -o - -ffreestanding -triple=i386-pc-linux-gn

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

efriedma added inline comments. Comment at: lib/CodeGen/CodeGenModule.cpp:2434 +// Emit section information for extern variables. +if (D->hasExternalStorage() && !D->isThisDeclarationADefinition()) { + if (const SectionAttr *SA = D->getAttr()) Why do

[PATCH] D36642: [Lexer] Report more precise skipped regions (PR34166)

efriedma added a comment. I'm not sure this produces the right locations in general. Consider the following slightly evil testcase: #\ if 0 #elif \ 11\ int a; #endif https://reviews.llvm.org/D36642 ___ cfe-commits mailing list cf

[PATCH] D36712: Emit section information for extern variables

efriedma added inline comments. Comment at: docs/LangRef.rst:629 +corresponding to the LLVM module, section information specified in the +declaration is retained in LLVM IR to enable OpenCL processes. + This doesn't really explain the part that matters. For Lang

[PATCH] D36712: Emit section information for extern variables

efriedma added a comment. > The other side of the problem is, what if the declarations don't have any > section information, but the definition does? Is that also an undefined > behavior? I can't see how "undefined behavior" could possibly be the right answer in that case. Every definition ha

[PATCH] D36642: [Lexer] Report more precise skipped regions (PR34166)

efriedma added inline comments. Comment at: lib/Lex/PPDirectives.cpp:570 + // We'll warn about reaching the end of file later. + if (C == '\0' || C == '\r' || C == '\n') +break; This doesn't really handle backslash-escaped newlines correctly. (

[PATCH] D36712: Emit section information for extern variables

efriedma added inline comments. Comment at: docs/LangRef.rst:628 +information. Attaching section information to an external declaration is an +assertion that it's definition is located in the specified section. If the +definition is located in a different section, the behavior

[PATCH] D36712: Emit section information for extern variables

efriedma added a comment. > If there is no section given explicitly, there is function > SelectSectionForGlobal that will determine the section based on the > properties of the global. If there is code that only sees the declaration, > but needs to know the section, it will get it from that fun

[PATCH] D36642: [Lexer] Report more precise skipped regions (PR34166)

efriedma added a comment. I'd like to see someone more familiar with indexing comment on the effect there; otherwise LGTM. https://reviews.llvm.org/D36642 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

efriedma added a comment. http://lab.llvm.org:8011/builders/aosp-O3-polly-before-vectorizer-unprofitable is currently broken, I think due to this patch. It looks like polly isn't getting linked into clang by default anymore? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D57054: [MachineOutliner][ARM][RFC] Add Machine Outliner support for ARM

efriedma added a comment. It might be possible to rearrange Low Overhead Loops to run before ConstantIslands, but you'd probably need to do more to make it work properly. I don't think ConstantIslands knows how to handle the branches generated by LowOverheadLoop. If you think it's necessary,

[PATCH] D71698: [AArch64][SVE] Add intrinsic for non-faulting loads

efriedma added inline comments. Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:5333 + // We need a layer of indirection because early machine code passes balk at + // physical register (i.e. FFR) uses that have no previous definition. + let hasSideEffects = 1, hasNoSch

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71082/new/ https://reviews.llvm.org/D71082 ___

[PATCH] D72467: [WIP] Remove "mask" operand from shufflevector.

efriedma updated this revision to Diff 237453. efriedma added a comment. Herald added subscribers: cfe-commits, Petar.Avramovic, dexonsmith, nhaehnle, jvesely, arsenm. Herald added a project: clang. Add more comments, fixed the GlobalISel representation of shuffles, misc other cleanups. Still h

[PATCH] D72467: [WIP] Remove "mask" operand from shufflevector.

efriedma marked an inline comment as done. efriedma added inline comments. Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:4413 unsigned MaxRecurse) { - if (isa(Mask)) + if (all_of(Mask, [](int Elem) { return Elem == -1; })) r

[PATCH] D72467: [WIP] Remove "mask" operand from shufflevector.

efriedma updated this revision to Diff 237463. efriedma added a comment. Figured out the changes necessary to get bitcode reading/writing working. ninja check now passes. Not sure what parts I should split off to get reviewed separately. Probably the GlobalISel changes could be split off with

[PATCH] D72612: [AArch64][SVE] Add ImmArg property to intrinsics with immediates

efriedma added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:1099 +class AsmVectorIndexOpnd +: Operand, ComplexPattern", []> { let ParserMatchClass = mc; sdesmalen wrote: > @efriedma @rengolin The idea here is to use a Compl

[PATCH] D72579: Evaluate __{c11_,}atomic_is_lock_free to 0 (avoid libcall) if larger than MaxAtomicPromoteWidth

efriedma added a comment. Eagerly evaluating based on MaxAtomicPromoteWidth seems fine... assuming we're actually setting MaxAtomicPromoteWidth to something appropriate. The value on PowerPC looks wrong. If we're worried about constant-evaluating it in contexts where gcc doesn't, we can gener

[PATCH] D72467: [WIP] Remove "mask" operand from shufflevector.

efriedma marked an inline comment as done. efriedma added inline comments. Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:4413 unsigned MaxRecurse) { - if (isa(Mask)) + if (all_of(Mask, [](int Elem) { return Elem == -1; })) r

[PATCH] D72467: Remove "mask" operand from shufflevector.

efriedma updated this revision to Diff 237825. efriedma retitled this revision from "[WIP] Remove "mask" operand from shufflevector." to "Remove "mask" operand from shufflevector.". efriedma added a comment. Rebase. Address review comment. More work on bitcode. I probably need to come up with

[PATCH] D72467: Remove "mask" operand from shufflevector.

efriedma updated this revision to Diff 237830. efriedma edited the summary of this revision. efriedma added a comment. Fixed one more issue. While I'm at it, get rid of the old overload of ConstantExpr::getShuffleVector, which only had a few remaining uses. Repository: rG LLVM Github Monorep

[PATCH] D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO

efriedma added a comment. Okay. Please let me know if you want me to review anything. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71387/new/ https://reviews.llvm.org/D71387 ___ cfe-commits mailing l

[PATCH] D67678: PR17164: Change clang's default behavior from -flax-vector-conversions=all to -flax-vector-conversions=integer.

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67678/new/ https://reviews.llvm.org/D67678 ___

[PATCH] D72612: [AArch64][SVE] Add ImmArg property to intrinsics with immediates

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM with one question Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:1108 + def "" : AsmVectorIndexOpnd, PatLeaf<(ty imm), pred>; + def _timm : AsmVectorIn

[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

efriedma accepted this revision. efriedma added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86621/new/ https://reviews.llvm.org/D86621 ___ cfe-commits mailing list cfe-commits@lists.llv

<    4   5   6   7   8   9   10   11   12   13   >