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

This revision was automatically updated to reflect the committed changes. Closed by commit rL307051: [Sema] Make BreakContinueFinder handle nested loops. (authored by efriedma). Changed prior to commit: https://reviews.llvm.org/D34568?vs=103761&id=105128#toc Repository: rL LLVM https://revi

[PATCH] D35008: [AArch64] Produce the right kind of va_arg for windows

efriedma added a comment. Needs a testcase (to check the emitted IR). Comment at: lib/CodeGen/TargetInfo.cpp:4769 DarwinPCS }; Can we extend ABIKind rather than adding a separate boolean? https://reviews.llvm.org/D35008 __

[PATCH] D35190: __builtin_constant_p should consider the parameter of a constexpr function as constant

efriedma added a comment. See https://bugs.llvm.org/show_bug.cgi?id=4898 for more discussion of __builtin_constant_p in LLVM vs. gcc. https://reviews.llvm.org/D35190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-

[PATCH] D34859: [COFF, ARM64] Set the data type widths and the data layout string for COFF ARM64

efriedma added inline comments. Comment at: lib/Basic/Targets.cpp:6662 +DoubleAlign = LongLongAlign = 64; +LongDoubleWidth = LongDoubleAlign = 64; +IntMaxType = SignedLongLong; If you're changing LongDoubleWidth and LongDoubleAlign, you also have to c

[PATCH] D34859: [COFF, ARM64] Set the data type widths and the data layout string

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

[PATCH] D40023: [RISCV] Implement ABI lowering

efriedma accepted this revision. efriedma added a comment. LGTM Not sure if anyone's mentioned it yet, but there's a C ABI testing tool in clang/utils/ABITest/ which you'll probably want to try at some point. Comment at: lib/CodeGen/TargetInfo.cpp:8913 + } + return getNatur

[PATCH] D40023: [RISCV] Implement ABI lowering

efriedma added a comment. Looks like it fails with assertions disabled; no-asserts builds disable value labels, so the IR output looks a little different. Repository: rL LLVM https://reviews.llvm.org/D40023 ___ cfe-commits mailing list cfe-commi

[PATCH] D40023: [RISCV] Implement ABI lowering

efriedma added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:8913 + } + return getNaturalAlignIndirect(Ty, /*ByVal=*/true); +} asb wrote: > efriedma wrote: > > asb wrote: > > > efriedma wrote: > > > > The spec says "Aggregates larger than 2✕XLEN bits

[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. /// RKSimon wrote: > efriedma wrote: > > craig.topper wrote:

[PATCH] D42154: Don't generate inline atomics for i386/i486

efriedma added a comment. The LLVM backend currently assumes every CPU is Pentium-compatible. If we're going to change that in clang, we should probably fix the backend as well. Comment at: lib/Basic/Targets/X86.h:472 +CPUKind Kind = getCPUKind(Opts.CPU); +if (Kind >=

[PATCH] D42249: [CodeGenCXX] annotate a GEP to a derived class with 'inbounds' (PR35909)

efriedma added inline comments. Comment at: lib/CodeGen/CGClass.cpp:410 + + // The GEP is to a derived object, so this GEP must be 'inbounds'. + Value = Builder.CreateInBoundsGEP(Value, Builder.CreateNeg(NonVirtualOffset), Not sure this comment really adds anyt

[PATCH] D42249: [CodeGenCXX] annotate a GEP to a derived class with 'inbounds' (PR35909)

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

[PATCH] D29817: [AVR] Fix __AVR_xxx macro definitions

efriedma added inline comments. Comment at: lib/Basic/Targets.cpp:8464 static ArrayRef AVRMcus = { - { "at90s1200", "__AVR_AT90S1200__" }, - { "attiny11", "__AVR_ATtiny11" }, - { "attiny12", "__AVR_ATtiny12" }, - { "attiny15", "__AVR_ATtiny15" }, - { "attiny28", "__AVR_ATti

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

efriedma added inline comments. Comment at: include/clang/Basic/Builtins.def:1091 +LIBBUILTIN(lgammaf, "ff", "fn", "math.h", ALL_LANGUAGES) +LIBBUILTIN(lgammal, "LdLd", "fn", "math.h", ALL_LANGUAGES) Please add a comment explaining why this doesn't have an "e"

[PATCH] D29723: [Sema] Add lvalue-to-rvalue cast in direct-list-initialization of enum

efriedma added a comment. (Resigning as a reviewer; I don't know enough about standard conversion sequences off the top of my head to review this properly.) https://reviews.llvm.org/D29723 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

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

efriedma added inline comments. Comment at: docs/LanguageExtensions.rst:2344 +declaration failed to receive the attribute because of a compilation error. The +attributes that aren't applied to any declaration are not verified semantically. + I think "to each dec

[PATCH] D29401: Fix MSVC Compatibility around dependent type with missing 'typename'

efriedma added inline comments. Comment at: lib/Parse/ParseStmt.cpp:186 // found. -if (Next.isNot(tok::coloncolon)) { +if (Next.isNot(tok::coloncolon) && (!getLangOpts().MSVCCompat || +Next.isNot(tok::less))) { erichkeane wrote: > Clang-tidy

[PATCH] D29401: Fix MSVC Compatibility around dependent type with missing 'typename'

efriedma added inline comments. Comment at: lib/Parse/ParseStmt.cpp:186 // found. -if (Next.isNot(tok::coloncolon)) { +if (Next.isNot(tok::coloncolon) && (!getLangOpts().MSVCCompat || +Next.isNot(tok::less))) { erichkeane wrote: > efriedma wr

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

efriedma added inline comments. Comment at: docs/LanguageExtensions.rst:2349 +attribute is supported by the pragma by referring to the +:doc:`individual documentation for that attribute `. arphaman wrote: > arphaman wrote: > > efriedma wrote: > > > I'm wondering

[PATCH] D29437: [ubsan] Detect signed overflow UB in remainder operations

efriedma added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:2380 + CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) && + Ops.Ty->isIntegerType()) { CodeGenFunction::SanitizerScope SanScope(&CGF); I don't think you need the isInt

[PATCH] D29437: [ubsan] Detect signed overflow UB in remainder operations

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

[PATCH] D26955: Fix bitwidth for x87 extended-precision floating-point type

efriedma added inline comments. Comment at: lib/Basic/TargetInfo.cpp:229 switch (BitWidth) { - case 96: + case 80: if (&getLongDoubleFormat() == &llvm::APFloat::x87DoubleExtended) ddcc wrote: > bruno wrote: > > The change makes sense but I believe there

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

efriedma added a comment. The logic to compute whether a calling convention uses SSE registers does exist in clang/lib/CodeGen/TargetInfo.cpp, so it might be possible to reuse that... but I'm not sure that's better than just detecting it in the backend. https://reviews.llvm.org/D27304 _

[PATCH] D26955: Fix bitwidth for x87 extended-precision floating-point type

efriedma added a comment. There's nothing wrong with this change, as far as I can tell. That said, if you're planning to use getRealTypeByWidth anywhere other than AddModeAttr, it's probably a bug; the behavior of getRealTypeByWidth isn't useful for any other purpose given that clang supports

[PATCH] D26843: Make sizeof expression context partially evaluated

efriedma added a comment. I would say make a new review thread since none of the discussion here is going to be helpful for understanding the new patch... but it really doesn't matter. https://reviews.llvm.org/D26843 ___ cfe-commits mailing list cf

[PATCH] D31856: Headers: Make the type of SIZE_MAX the same as size_t

efriedma added a comment. We normally use stdint.h from the host C library, rather than our own version; this file is only relevant in -ffreestanding mode. So it should be safe to change. https://reviews.llvm.org/D31856 ___ cfe-commits mailing li

[PATCH] D31856: Headers: Make the type of SIZE_MAX the same as size_t

efriedma added inline comments. Comment at: clang/test/Headers/stdint-typeof-MINMAX.cpp:1 +// RUN: %clang_cc1 %s -ffreestanding -std=c++1z -fsyntax-only -triple=aarch64-none-none +// RUN: %clang_cc1 %s -ffreestanding -std=c++1z -fsyntax-only -triple=arm-none-none --

[PATCH] D31856: Headers: Make the type of SIZE_MAX the same as size_t

efriedma accepted this revision. efriedma added a comment. This revision now requires changes to proceed. LGTM. https://reviews.llvm.org/D31856 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cf

[PATCH] D32456: [ubsan] Make the alignment check work with some extern decls (PR32630)

efriedma added a comment. > Note: it would still not be able to catch the following case -- Do you know why? Comment at: lib/CodeGen/CodeGenModule.cpp:2319 + // The alignment sanitizer can catch more bugs if we don't attach + // alignment info to extern globals. +

[PATCH] D32456: [ubsan] Make the alignment check work with some extern decls (PR32630)

efriedma added a comment. > If the alignment isn't explicitly set, it is initialized to 0. That's what I thought. I don't like this; clang should explicitly set an alignment for every global. https://reviews.llvm.org/D32456 ___ cfe-commits mailin

[PATCH] D32456: [ubsan] Make the alignment check work with some extern decls (PR32630)

efriedma added a comment. Err, that's not what I meant... If the alignment of a global in LLVM IR is "zero", it doesn't really mean zero. It means "guess the alignment I want based on the specified type of the global". And that's not a game we ever want to play when generating IR from clang.

[PATCH] D32456: [ubsan] Make the alignment check work with some extern decls (PR32630)

efriedma added a comment. This should work correctly with alignment attributes, I think? IIRC those just change the return value of getDeclAlign(). I agree it's a little fragile, but I don't have a good suggestion to avoid that. https://reviews.llvm.org/D32456 _

[PATCH] D32759: Fix errored return value in CheckFunctionReturnType and add a fixit hint

efriedma added a comment. The difference between returning true and false here is just the way error recovery works: when we return true, we know the type is invalid, so we suppress it, and subsequent errors involving the declaration. Example (Objective-C++) where we currently print two errors

[PATCH] D32759: Fix errored return value in CheckFunctionReturnType and add a fixit hint

efriedma added a comment. In https://reviews.llvm.org/D32759#748653, @chenwj wrote: > In https://reviews.llvm.org/D32759#748007, @efriedma wrote: > > > The difference between returning true and false here is just the way error > > recovery works: when we return true, we know the type is invalid,

[PATCH] D32759: Fix errored return value in CheckFunctionReturnType and add a fixit hint

efriedma added a comment. A testcase would be nice... but otherwise, yes. https://reviews.llvm.org/D32759 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32759: Fix errored return value in CheckFunctionReturnType and add a fixit hint

efriedma added a comment. The testcase I wrote should change behavior, I think. https://reviews.llvm.org/D32759 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32759: Fix errored return value in CheckFunctionReturnType and add a fixit hint

efriedma added a comment. Our behavior for your testcase is less than ideal (we should be generating an invalid declaration instead of just dropping it on the floor), but that's not something you need to fix here. Could you also include the Objective-C++ testcase I wrote? Or does it not work

[PATCH] D32759: Fix errored return value in CheckFunctionReturnType and add a fixit hint

efriedma added a comment. Err, I meant the one I wrote earlier in this thread: https://reviews.llvm.org/D32759#748007 https://reviews.llvm.org/D32759 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D32759: Fix errored return value in CheckFunctionReturnType and add a fixit hint

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

[PATCH] D32759: Fix errored return value in CheckFunctionReturnType and add a fixit hint

efriedma added a comment. Err, oh, meant to add one minor comment: please rename "method-bad-param.mm" to "interface-return-type.mm". https://reviews.llvm.org/D32759 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-

[PATCH] D91607: [clang][Sparc] Fix __builtin_extract_return_addr etc.

efriedma added a comment. Testcase? Do you need to ptrtoint/inttoptr? I would expect that the address is an `i8*`, so you can just GEP an appropriate number of bytes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91607/new/ https://reviews.llvm.

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

efriedma added inline comments. Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:135 } +// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with +// libatomic as a workaround. glaubitz wrote: > glaubitz wrote: > > ro wrote: > > > ro wr

[PATCH] D91607: [clang][Sparc] Fix __builtin_extract_return_addr etc.

efriedma added a comment. In D91607#3283350 , @ro wrote: > In D91607#3280808 , @efriedma wrote: > >> Testcase? > > I thought the 3 testcases adjusted in D91608 > to use `__builtin_

[PATCH] D91607: [clang][Sparc] Fix __builtin_extract_return_addr etc.

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/D91607/new/ https://reviews.llvm.org/D91607 _

[PATCH] D120323: [clang][SVE] Add support for arithmetic operators on SVE types

efriedma added a comment. Is there a corresponding ARM specification update for this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120323/new/ https://reviews.llvm.org/D120323 ___ cfe-commits mailing li

[PATCH] D120936: [Sema][Windows] Don't special-case void* in __unaligned conversions.

efriedma created this revision. efriedma added reviewers: rnk, mstorsjo, andreybokhanko, majnemer, aaron.ballman. Herald added a project: All. efriedma requested review of this revision. Herald added a project: clang. As far as I can tell, MSVC allows the relevant conversions for all pointer typ

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

efriedma added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:10723-10733 + bool LHSIsNullPtr = LHS.get()->IgnoreParenCasts()->isNullPointerConstant( + Context, Expr::NPC_ValueDependentIsNotNull); + bool RHSIsNullPtr = RHS.get()->IgnoreParenCasts()->i

[PATCH] D101873: [clang] Support clang -fpic -fno-semantic-interposition for AArch64

efriedma added a comment. In D101873#2741903 , @peter.smith wrote: > Looking at the gist I've got one concern for AArch64 and Arm. The ABI relies > on thunks which are only defined for symbols of type STT_FUNC. Changing > branches to STT_FUNC to STT_SE

[PATCH] D102015: [clang CodeGen] Don't crash on large atomic function parameter.

efriedma created this revision. efriedma added reviewers: rsmith, ahatanak, rjmccall. Herald added a subscriber: jfb. efriedma requested review of this revision. Herald added a project: clang. I wouldn't recommend writing code like the testcase; a function parameter isn't atomic, so using an atom

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

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

[PATCH] D102015: [clang CodeGen] Don't crash on large atomic function parameter.

efriedma added a comment. In D102015#2743634 , @rjmccall wrote: > Objective-C object types also have aggregate evaluation kind. Those can only > be used as value types in an old, fragile ObjC ABI, but I believe we haven't > yet formally decided to remo

[PATCH] D102015: [clang CodeGen] Don't crash on large atomic function parameter.

efriedma added a comment. In D102015#2748441 , @efriedma wrote: >> ...I'm confused about why this code is doing what it's doing with cleanups, >> though. Why does it only apply when the parameter is indirect? I believe >> `isParamDestroyedInCallee()`

[PATCH] D102015: [clang CodeGen] Don't crash on large atomic function parameter.

efriedma updated this revision to Diff 344193. efriedma added a comment. Use isRecordType() instead of checking for an atomic type. Fix the caller side as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102015/new/ https://reviews.llvm.org/D1

[PATCH] D100388: [BROKEN][clang] Try to fix thunk function types

efriedma added a comment. Made a couple suggestions to make this easier to review. The test changes you've made so far seem reasonable. Is there some specific section of the code you want feedback on? Comment at: clang/include/clang/Basic/Thunk.h:1 +//===- Thunk.h - Decla

[PATCH] D102015: [clang CodeGen] Don't crash on large atomic function parameter.

efriedma added a comment. > Using _Atomic for C structs with non-trivially destructible fields currently > doesn't work, so maybe that should just be disallowed. I'd prefer to disallow _Atomic on all types that aren't trivially destructible, yes. Not impossible to support, of course, but I don

[PATCH] D116059: [Clang][CFG] check children statements of asm goto

efriedma added a comment. Do we have testcases for how -Wuninitialized/-Wmaybe-uninitialized interact with asm goto? If not, can you add them? Also, an explicit test for -Warray-bounds or whatever would be nice, although I guess the CFG tests sort of cover that. Repository: rG LLVM Github

[PATCH] D116059: [Clang][CFG] check children statements of asm goto

efriedma added a comment. Yes, messed up the warning flag. Did some brief experiments on trunk; I'd like to see the following two testcases as regression tests: int a(int z) { int x; if (z) asm goto ("":"=r"(x):::A1,A2); return 0; A1: A2: return x;

[PATCH] D116059: [Clang][CFG] check children statements of asm goto

efriedma accepted this revision. efriedma added a comment. LGTM Comment at: clang/test/Analysis/uninit-asm-goto.cpp:84 + +int test8() { +int x = 0; // expected-warning {{variable 'x' is used uninitialized whenever its declaration is reached}} Looks like th

[PATCH] D116833: [clang] Introduce support for disabling warnings in system macros

efriedma added a comment. I'll just note here that doing this globally is likely to have unexpected results... consider, for example: #include void f() { long x = M_PI; } Currently, the implicit conversion warning points into math.h. That said, I don't see any problem with the current imp

[PATCH] D27800: Add overload of TransformToPotentiallyEvaluated for TypeSourceInfo

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. Looks like all the review comments have been addressed. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D27800/new/ https://reviews.llvm.

[PATCH] D115942: [X86][MS] Change the alignment of f80 to 16 bytes on Windows 32bits to match with ICC

efriedma added a comment. Not sure I agree with the "fix" to the lld tests. If the linker is crashing, clearly there's a bug, not just an incorrect test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115942/new/ https://reviews.llvm.org/D115942

[PATCH] D41316: [libcxx] Allow random_device to be built optionally

efriedma abandoned this revision. efriedma added a comment. Herald added a subscriber: libcxx-commits. A very similar patch was merged in e0d01294bc124211a8ffb55e69162eb34a242680 . Repository: rCXX libc++ CHANGES SINCE LAS

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

efriedma added inline comments. Comment at: llvm/test/Transforms/LoopVectorize/PowerPC/optimal-epilog-vectorization.ll:273-276 -; VF-TWO-CHECK-DAG: [[LOOPID_MV]] = distinct !{[[LOOPID_MV]], [[LOOPID_DISABLE_VECT:!.*]]} -; VF-TWO-CHECK-DAG: [[LOOPID_EV]] = distinct !{[[LOOPID_EV

[PATCH] D115670: Correct behavior of Vector boolean-operations, implement vector operator-

efriedma added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:2915 case BO_EQ: -Result = (LHSValue == RHSValue); +Result = -(LHSValue == RHSValue); break; Using "operator=" to assign an int to an APInt is going to lead to weird/confu

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

efriedma added a comment. From earlier review comment: > Missing changes to run the unittests (test/builtins/Unit/) for the new > functions. That's the most important part, so we have some confidence the code actually works. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D116059: [Clang][CFG] check children statements of asm goto

efriedma added inline comments. Comment at: clang/lib/Analysis/UninitializedValues.cpp:826 +// it as potentially uninitialized for those cases where it's used on +// an indirect path, where it's not guaranteed to be defined. vals[VD] = MayUninitialized; -

[PATCH] D98363: [Sema] Fold VLA types in compound literals to constant arrays.

efriedma added inline comments. Comment at: clang/test/Sema/vla.c:134 + // expected-warning@+1{{variable length array folded to constant array as an extension}} + char (*a9)[] = (char[2][ksize]) {{1,2,3,4},{4,3,2,1}}; + aaron.ballman wrote: > efriedma wrote: >

[PATCH] D99791: [CGCall] Annotate pointer argument with alignment

efriedma added a comment. This feels scary: the C standard technically allows this, but we haven't done it in the past, and it could break otherwise functioning code. (We've only assumed alignment about pointers that are dereferenced/dereferenceable.) For D99790

[PATCH] D99675: RFC [llvm][clang] Create new intrinsic llvm.arith.fence to control FP optimization at expression level

efriedma added a comment. > The expression “llvm.arith.fence(a * b) + c” means that “a * b” must happen > before “+ c” and FMA guarantees that, but to prevent later optimizations from > unpacking the FMA the correct transformation needs to be: > > llvm.arith.fence(a * b) + c → llvm.arith.fence

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

efriedma added a comment. Please add a test for `(char*)0-(char*)0`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98798/new/ https://reviews.llvm.org/D98798 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-b

[PATCH] D99675: RFC [llvm][clang] Create new intrinsic llvm.arith.fence to control FP optimization at expression level

efriedma added a comment. In D99675#2672138 , @kbsmith1 wrote: > In D99675#2671924 , @efriedma wrote: > >> How is llvm.arith.fence() different from using "freeze" on a floating-point >> value? The goal isn't reall

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

efriedma added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:10724 + // Subtracting from a null pointer should produce a warning. + if (LHS.get()->IgnoreParenCasts()->isNullPointerConstant( + Context, Expr::NPC_ValueDependentIsNotNull)) -

[PATCH] D98363: [Sema] Fold VLA types in compound literals to constant arrays.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdc1ab590a052: [Sema] Fold VLA types in compound literals to constant arrays. (authored by efriedma). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D107882: BPF: Enable frontend constant folding for VLA size

efriedma added a comment. I don't think my opinion has changed here. I'm against the solution proposed in this patch. The other solutions discussed in the review seem fine. (The simplest is just to make the bpf backend ignore stacksave/stackrestore calls.) Repository: rG LLVM Github Monor

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

efriedma added a comment. errno handling for math library functions is a mess. Currently, we don't model it properly; we just mark the calls "readnone" and hope for the best. If you don't want to fix that, just check for readnone for now. I don't think we want to be querying function attribut

[PATCH] D101759: [PowerPC] Scalar IBM MASS library conversion pass

efriedma added a comment. > I agree. I think the problem is that this patch is trying to decide on a > global lowering strategy for llvm.* math intrinsics in > llvm/lib/Target/PowerPC/PPCISelLowering.cpp but such global decision making > does not go well with finer granularity of fast-math flag

[PATCH] D107882: BPF: Enable frontend constant folding for VLA size

efriedma added a comment. I'd prefer not to mess with the AST if we don't need to; more differences between targets make it harder to understand any issues that come up. BPF-flavored C already has enough weird differences without adding unnecessary changes. If all you need is to avoid unneces

<    13   14   15   16   17   18