[PATCH] D59287: [X86] Only define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 in 64-bit mode.

jfb accepted this revision. jfb added a comment. This revision is now accepted and ready to land. OK, that seems unfortunate but unlikely and consistency terrible with GCC. Let's do it. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59287/new/

[PATCH] D55895: NFC: simplify Darwin environment handling

jfb added a comment. Herald added a project: clang. In D55895#1336777 , @arphaman wrote: > I think we might need to run it past the internal builds to see if anything > breaks. We've been running it for a while without breakage. Good to go?

[PATCH] D58885: Variable auto-init: split out small arrays

This revision was automatically updated to reflect the committed changes. Closed by commit rL355660: Variable auto-init: split out small arrays (authored by jfb, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST

[PATCH] D59523: Thread Safety: also look at ObjC methods

jfb marked an inline comment as done. jfb added inline comments. Comment at: test/SemaObjCXX/no-crash-thread-safety-analysis.mm:1 +// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wno-objc-root-class %s + aaron.ballman wrote: > aaronpuchert wrote: > >

[PATCH] D59523: Thread Safety: also look at ObjC methods

jfb marked an inline comment as done. jfb added inline comments. Comment at: test/SemaObjCXX/no-crash-thread-safety-analysis.mm:1 +// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wno-objc-root-class %s + aaron.ballman wrote: > jfb wrote: > >

[PATCH] D57797: Variable auto-init: fix __block initialization

jfb updated this revision to Diff 185883. jfb added a comment. - Only initialize __block's storage. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57797/new/ https://reviews.llvm.org/D57797 Files: lib/CodeGen/CGDecl.cpp

[PATCH] D57797: Variable auto-init: fix __block initialization

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rC353495: Variable auto-init: fix __block initialization (authored by jfb, committed by ). Changed prior to commit:

[PATCH] D57797: Variable auto-init: fix __block initialization

jfb marked 3 inline comments as done. jfb added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1643 CharUnits Size = getContext().getTypeSizeInChars(type); if (!Size.isZero()) { switch (trivialAutoVarInit) { rjmccall wrote: > jfb wrote: > >

[PATCH] D57797: Variable auto-init: fix __block initialization

jfb marked 2 inline comments as done. jfb added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1605 -emitByrefStructureInit(emission); - // Initialize the variable here if it doesn't have a initializer and it is a rjmccall wrote: > Are these changes

[PATCH] D57797: Variable auto-init: fix __block initialization

jfb marked 2 inline comments as done. jfb added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1726 +emitByrefStructureInit(emission); + } + Note that we still want this to be pulled out in this way because `emitByrefStructureInit` emits the call to

[PATCH] D57797: Variable auto-init: fix __block initialization

jfb updated this revision to Diff 185892. jfb marked 4 inline comments as done. jfb added a comment. - Simplify patch greatly. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57797/new/ https://reviews.llvm.org/D57797 Files: lib/CodeGen/CGDecl.cpp

[PATCH] D57797: Variable auto-init: fix __block initialization

jfb updated this revision to Diff 185896. jfb added a comment. - Remove whitespace changes. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57797/new/ https://reviews.llvm.org/D57797 Files: lib/CodeGen/CGDecl.cpp test/CodeGenCXX/trivial-auto-var-init.cpp

[PATCH] D57797: Variable auto-init: fix __block initialization

jfb marked an inline comment as done. jfb added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1642 CharUnits Size = getContext().getTypeSizeInChars(type); if (!Size.isZero()) { jfb wrote: > rjmccall wrote: > > Does this check handle flexible

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

jfb added a comment. Can you add a link to bug 40605 in the commit message? > I'm not quite sure how to show the resulting difference in code. Do you mean > we need Clang to support both modes and to compare the resulting assembly? I only meant tests that show codegen, as you've now added

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

jfb added a comment. In D57898#1390816 , @glider wrote: > > Can you add a link to bug 40605 in the commit message? > > It's in the title now, doesn't that count? :) Oh OK I hadn't seen that. CHANGES SINCE LAST ACTION

[PATCH] D44248: [clangd][cmake] Provide libatomic when there is no native support for 64bit atomics

jfb added a comment. In D44248#1391798 , @matthewbauer wrote: > This appears to be adding atomic on macOS builds, which shouldn't be > happening: > > https://hydra.nixos.org/build/88290234/ > > HAVE_CXX_ATOMICS64_WITHOUT_LIB is never being set. Odd.

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

jfb added a comment. Overall this LGTM besides a few nits, and wanting input from @rjmccall. As follow-ups (which I can take on): - Handle the case where the types don't match (because `Loc` was adjusted). - Handle small arrays (and any other cases where the struct stores get broken down into

[PATCH] D58164: Block+lambda: allow reference capture

jfb created this revision. jfb added reviewers: rjmccall, ahatanak. Herald added subscribers: cfe-commits, dexonsmith, jkorous. Herald added a project: clang. Capturing a C++ object by reference wasn't quite working when mixing block and lambda. Repository: rC Clang

[PATCH] D58164: Block+lambda: allow reference capture

jfb added a comment. I talked to Akira in meatspace, and it seems like this updated patch does the right thing. He suggested changing the AST as a longer-term solution, but for now this approach seems simple enough. Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D58164: Block+lambda: allow reference capture

jfb updated this revision to Diff 186752. jfb added a comment. - Check for references when looking for copyexpr directly. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58164/new/ https://reviews.llvm.org/D58164 Files: lib/CodeGen/CGBlocks.cpp

[PATCH] D58149: [clang] Make sure C99/C11 features in are provided in C++11

jfb added a comment. In D58149#1397382 , @efriedma wrote: > Formally, I don't think C11 is a normative reference for C++11 or C++14, only > C++17 (see [intro.refs] in the standard). Right, this was changed in wg21.link/p0063r3 That being said, we *can*

[PATCH] D58218: Variable auto-init of blocks capturing self after init bugfix

jfb created this revision. jfb added a reviewer: ahatanak. Herald added subscribers: cfe-commits, dexonsmith, jkorous. Herald added a project: clang. Blocks that capture themselves (and escape) after initialization currently codegen wrong because this: bool capturedByInit = Init &&

[PATCH] D58218: Variable auto-init of blocks capturing self after init bugfix

jfb marked an inline comment as done. jfb added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1640 // Only initialize a __block's storage: we always initialize the header. -if (emission.IsEscapingByRef) +if (emission.IsEscapingByRef && isa(Loc.getPointer()))

[PATCH] D58149: [clang] Make sure C99/C11 features in are provided in C++11

jfb added a comment. In D58149#1397532 , @rsmith wrote: > In D58149#1397499 , @ldionne wrote: > > > In D58149#1397390 , @jfb wrote: > > > > > In D58149#1397382

[PATCH] D58218: Variable auto-init of blocks capturing self after init bugfix

jfb added a comment. In D58218#1398096 , @rjmccall wrote: > Well, you can always make a variable with a more directly-applicable name > than `capturedByInit` and update it as appropriate, like `locIsByrefHeader`. Sounds good. I made it `const` too, to

[PATCH] D58218: Variable auto-init of blocks capturing self after init bugfix

jfb updated this revision to Diff 186853. jfb added a comment. - Update with John's suggestion. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58218/new/ https://reviews.llvm.org/D58218 Files: lib/CodeGen/CGDecl.cpp test/CodeGenCXX/trivial-auto-var-init.cpp

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

jfb added a comment. In D57898#1395953 , @glider wrote: > ... which happily skips the padding. I don't think padding is an issue right now. It's valid to either initialize it or not. It is slightly unfortunate to lose the information about padding

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

jfb added inline comments. Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:979 + if (CGM.getCodeGenOpts().OptimizationLevel == 0) +return false; + if (GlobalSize <= SizeLimit) rjmccall wrote: > glider wrote: > > jfb wrote: > > > The general 64-byte

[PATCH] D58218: Variable auto-init of blocks capturing self after init bugfix

jfb updated this revision to Diff 186950. jfb added a comment. - Fix polarity Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58218/new/ https://reviews.llvm.org/D58218 Files: lib/CodeGen/CGDecl.cpp test/CodeGenCXX/trivial-auto-var-init.cpp Index:

[PATCH] D58218: Variable auto-init of blocks capturing self after init bugfix

jfb added a comment. In D58218#1398251 , @rjmccall wrote: > In D58218#1398124 , @jfb wrote: > > > In D58218#1398096 , @rjmccall > > wrote: > > > > > Well, you can always

[PATCH] D58218: Variable auto-init of blocks capturing self after init bugfix

This revision was automatically updated to reflect the committed changes. Closed by commit rL354147: Variable auto-init of blocks capturing self after init bugfix (authored by jfb, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D57797: Variable auto-init: fix __block initialization

jfb marked 5 inline comments as done. jfb added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1642 CharUnits Size = getContext().getTypeSizeInChars(type); if (!Size.isZero()) { rjmccall wrote: > Does this check handle flexible arrays on

[PATCH] D57797: Variable auto-init: fix __block initialization

jfb created this revision. jfb added reviewers: rjmccall, pcc, kcc. Herald added subscribers: cfe-commits, dexonsmith, jkorous. Herald added a project: clang. Automatic initialization [1] of __block variables was happening too late, which caused self-init usage to crash, such as here: typedef

[PATCH] D54604: Automatic variable initialization

jfb added a comment. In D54604#1375514 , @glider wrote: > Not sure if a similar problem was mentioned already or not, but the following > program: Yes please file a bug. Seems like the optimizer should sink the store enough that it appears only once

[PATCH] D60548: Variable auto-init: also auto-init alloca

jfb added a comment. One downside to alloca is that we can's use `__attribute__((uninitialized))` because it's a builtin. Maybe we need a separate uninitialized alloca? What do you all think? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60548/new/

[PATCH] D60548: Variable auto-init: also auto-init alloca

jfb created this revision. Herald added subscribers: cfe-commits, dexonsmith, jkorous. Herald added a project: clang. alloca isn’t auto-init’d right now because it’s a different path in clang that all the other stuff we support (it’s a builtin, not an expression). Interestingly, alloca doesn’t

[PATCH] D60548: Variable auto-init: also auto-init alloca

This revision was automatically updated to reflect the committed changes. Closed by commit rC358243: Variable auto-init: also auto-init alloca (authored by jfb, committed by ). Changed prior to commit: https://reviews.llvm.org/D60548?vs=194762=194787#toc Repository: rC Clang CHANGES SINCE

[PATCH] D60548: Variable auto-init: also auto-init alloca

jfb added a comment. I got an lgtm from @pcc on IRC, so I'll commit this. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60548/new/ https://reviews.llvm.org/D60548 ___ cfe-commits mailing list

[PATCH] D60548: Variable auto-init: also auto-init alloca

jfb added a comment. In D60548#1462124 , @jfb wrote: > One downside to alloca is that we can's use `__attribute__((uninitialized))` > because it's a builtin. Maybe we need a separate uninitialized alloca? What > do you all think? Actually I'm

[PATCH] D60548: Variable auto-init: also auto-init alloca

jfb added a comment. In D60548#1463181 , @jfb wrote: > In D60548#1462124 , @jfb wrote: > > > One downside to alloca is that we can's use > > `__attribute__((uninitialized))` because it's a builtin. Maybe we need a

[PATCH] D60548: Variable auto-init: also auto-init alloca

jfb updated this revision to Diff 194736. jfb added a comment. Herald added a subscriber: mgorny. - Move patternFor to a shared file as requested by @rjmccall Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60548/new/ https://reviews.llvm.org/D60548 Files:

[PATCH] D60548: Variable auto-init: also auto-init alloca

jfb marked 2 inline comments as done. jfb added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:57 + case LangOptions::TrivialAutoVarInitKind::Pattern: +Byte = CGF.Builder.getInt8(0xAA); +break; rjmccall wrote: > Can this value be taken from some

[PATCH] D60548: Variable auto-init: also auto-init alloca

jfb updated this revision to Diff 194762. jfb marked 2 inline comments as done. jfb added a comment. - Change name, qualify declaration. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60548/new/ https://reviews.llvm.org/D60548 Files: lib/CodeGen/CGBuiltin.cpp

[PATCH] D60548: Variable auto-init: also auto-init alloca

jfb marked 2 inline comments as done. jfb added inline comments. Comment at: lib/CodeGen/PatternInit.h:22 + +llvm::Constant *patternFor(CodeGenModule&, llvm::Type*); + rjmccall wrote: > Please choose names that mean something outside of the mental context you >

[PATCH] D60548: Variable auto-init: also auto-init alloca

jfb marked an inline comment as done. jfb added inline comments. Comment at: lib/CodeGen/PatternInit.h:22 + +llvm::Constant *patternFor(CodeGenModule&, llvm::Type*); + rjmccall wrote: > jfb wrote: > > rjmccall wrote: > > > Please choose names that mean something

[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

jfb added a comment. In D59307#1428101 , @mibintc wrote: > In D59307#1427644 , @jfb wrote: > > > I think you also want to test C++ `std::atomic` ... > > > The bug described in

[PATCH] D61923: [GWP-ASan] Mutex implementation [2].

jfb added a comment. > In D61923#1502404 , @jfb wrote: > >> > We can't use `std::mutex` as we must be C-compatible >> >> Can you clarify what you mean by this? I don't understand. >> Have you asked on libcxx-dev? Is there interest in the libc++ community

[PATCH] D62962: Clang implementation of sizeless types

jfb added a comment. How do these types mangle? Comment at: test/Sema/sizeless-1.c:60 + (void)__atomic_is_lock_free(1, _int8); + (void)__atomic_always_lock_free(1, _int8); + What do you expect these to return? Repository: rC Clang CHANGES SINCE LAST

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

jfb added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:5447 + +case APValue::LValue: { + LValue LVal; rsmith wrote: > This will permit bitcasts from `nullptr_t`, providing a zero value. I think > that's wrong; the bit representation of

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

jfb added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9716 +def err_bit_cast_non_trivially_copyable : Error< + "__builtin_bit_cast %select{source|destination}0 type must be a trivially copyable">; +def err_bit_cast_type_size_mismatch : Error<

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

jfb added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:5447 + +case APValue::LValue: { + LValue LVal; rsmith wrote: > jfb wrote: > > rsmith wrote: > > > This will permit bitcasts from `nullptr_t`, providing a zero value. I > > > think

[PATCH] D63518: WIP BitStream reader: propagate errors

jfb added a comment. This is still a work in progress. I still have to debug clang tests that fail, and compile / fix more than clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63518/new/ https://reviews.llvm.org/D63518

[PATCH] D63518: WIP BitStream reader: propagate errors

jfb added a comment. `check-clang` now passes all tests, so the patch is pretty much ready to review. I'll get started on the other parts of LLVM that use this API. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63518/new/

[PATCH] D63518: WIP BitStream reader: propagate errors

jfb added a comment. All of `check-llvm` now passes. I'll look at other users of this stuff in the monorepo, compile and run their tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63518/new/ https://reviews.llvm.org/D63518

[PATCH] D63518: BitStream reader: propagate errors

jfb added a comment. I did a build of all LLVM monorepo projects, and only LLVM / clang / clang-tools-extra are impacted. I therefore ran tests as follows, and they all pass: rm -rf debug ; mkdir debug && (cd debug && cmake -G Ninja ../llvm -DLLVM_ENABLE_ASSERTIONS=ON

[PATCH] D63518: BitStream reader: propagate errors

jfb marked 8 inline comments as done. jfb added inline comments. Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:527-529 + if (llvm::Error Err = readSubBlock(BlockOrCode, I)) { +if (llvm::Error Skipped = Stream.SkipBlock()) + return Skipped;

[PATCH] D63518: BitStream reader: propagate errors

jfb added a comment. In D63518#1558197 , @lhames wrote: > I haven't had a chance to audit the whole patch yet, but in general the error > suppression idioms are unsafe (though maybe no more so than the existing > code?). > > I would be inclined to audit

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

jfb added a comment. In D62825#1542377 , @rsmith wrote: > (You might argue that it's ridiculous to require that `nullptr_t` have the > same size and alignment as `void*` but not have the same storage > representation as a null `void*`. I'd agree, and

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added a comment. What does the suggested for for `2 ^ 32` look like? I hope it's not `1 << 32` :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 ___ cfe-commits mailing list

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added a comment. I think you want to avoid warning inside macros as well, say: #define AWESOME(x, y) ({ blah blah blah; x ^ y; blah }) AWESOME(2, 10); // probably not a bug Comment at: test/SemaCXX/warn-xor-as-pow.cpp:13 +res = a ^ b; +res = 2 ^ 0; +res

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added a comment. As a reference: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90885 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added a comment. Why have `.c` and `.cpp` tests? Comment at: test/Sema/warn-xor-as-pow.c:43 + res = TWO_ULL ^ 16; + res = 2 ^ 32; // expected-warning {{result of '2 ^ 32' is 34; maybe you mean '1<<32', but shift count >= width of type}} + // expected-note@-1 {{replace

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added inline comments. Comment at: test/Sema/warn-xor-as-pow.c:45 + // expected-note@-1 {{replace expression with '(2) ^ 32' to silence this warning}} + res = 2 ^ 64; // expected-warning {{result of '2 ^ 64' is 66; maybe you mean '1<<64', but shift count >= width of

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added a comment. In D63423#1550705 , @aaron.ballman wrote: > In D63423#1550697 , @jfb wrote: > > > What I meant with macros was that I don't think we should warn on: > > > > #define LEGIT(a, b) ({ work work

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added a comment. In D63423#1550725 , @lebedev.ri wrote: > I've always been frustrated at how clang just gives up as soon as it sees a > macro. > At best that should be controlled with some command-line flag. This patch is not the place to change

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added a comment. What I meant with macros was that I don't think we should warn on: #define LEGIT(a, b) ({ work work work; a ^ b; work work work; }) LEGIT(10, 5); If the constants are inline in the macros then sure, warn there. Basically: if you literally wrote `CONSTANT ^ CONSTANT`

[PATCH] D63518: WIP BitStream reader: propagate errors

jfb added a comment. In D63518#1550827 , @bruno wrote: > Hi JF. Thanks for working on this, nice improvement to error handling! > > The overall approach is pretty solid and should prevent a lot of red herring > while investigating hard to reproduce

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added inline comments. Comment at: include/clang/Basic/DiagnosticGroups.td:508 def Varargs : DiagGroup<"varargs">; +def XorUsedAsPow : DiagGroup<"xor-used-as-pow">; Does this match the naming that GCC ended up with? Comment at:

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10929 +// Do not diagnose if xor keyword is used. +if (ExprStr.find("xor") != llvm::StringRef::npos) + return; xbolva00 wrote: > Quuxplusone wrote: > > xbolva00 wrote: > > > jfb wrote: > >

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10929 +// Do not diagnose if xor keyword is used. +if (ExprStr.find("xor") != llvm::StringRef::npos) + return; aaron.ballman wrote: > xbolva00 wrote: > > xbolva00 wrote: > > > jfb wrote: >

[PATCH] D61879: WIP: Prototype of DSE optimizations for -ftrivial-auto-var-init

jfb added a comment. Hello! I'm still interested in this, have you been able to make any progress? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61879/new/ https://reviews.llvm.org/D61879 ___

[PATCH] D62962: Clang implementation of sizeless types

jfb added inline comments. Comment at: test/Sema/sizeless-1.c:66 + _Static_assert(__atomic_always_lock_free(1, _int8) == __atomic_always_lock_free(1, incomplete_ptr), ""); + _Static_assert(__atomic_always_lock_free(2, _int8) == __atomic_always_lock_free(2, incomplete_ptr),

[PATCH] D62960: SVE opaque type for C intrinsics demo

jfb added subscribers: rjmccall, jfb. jfb added a comment. Tests? Comment at: lib/AST/ItaniumMangle.cpp:2680 +break; +#include "clang/Basic/AArch64SVEACLETypes.def" } @rjmccall you probably should review this part. Repository: rC Clang CHANGES

[PATCH] D62962: Clang implementation of sizeless types

jfb added inline comments. Comment at: test/Sema/sizeless-1.c:66 + _Static_assert(__atomic_always_lock_free(1, _int8) == __atomic_always_lock_free(1, incomplete_ptr), ""); + _Static_assert(__atomic_always_lock_free(2, _int8) == __atomic_always_lock_free(2, incomplete_ptr),

[PATCH] D61923: [GWP-ASan] Mutex implementation [2].

jfb added a comment. In D61923#1502323 , @hctim wrote: > In D61923#1502245 , @jfb wrote: > > > Seems a shame to duplicate mutex again... Why can't use use the STL's > > version again? It doesn't allocate. > > > We

[PATCH] D60593: [GwpAsan] Introduce GWP-ASan.

jfb added a comment. In D60593#1498622 , @hctim wrote: > In D60593#1495428 , @gribozavr wrote: > > > What does GWP stand for? > > > Google Wide Profiling >

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

jfb added a comment. Follow-up test fix in http://llvm.org/r359636 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61280/new/ https://reviews.llvm.org/D61280 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

This revision was automatically updated to reflect the committed changes. Closed by commit rC359628: Variable auto-init: dont initialize aggregate padding of all aggregates (authored by jfb, committed by ). Changed prior to commit: https://reviews.llvm.org/D61280?vs=197188=197476#toc

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

jfb added a comment. This seems uncontroversial, I'm happy to make follow-up change if you have suggestions. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61280/new/ https://reviews.llvm.org/D61280 ___ cfe-commits

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

jfb added a comment. In D61280#1485073 , @rjmccall wrote: > IIRC, C says *members* are initialized as if they were in static storage, > which might mean that their interior padding should be zeroed, but I don't > think says anything about the padding in

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

jfb added a comment. In D61280#1485115 , @rjmccall wrote: > I don't think the implication is supposed to be that padding is > zero-initialized or not depending on where in the aggregate it appears, but > it doesn't really matter, I don't think we're

[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

jfb added a comment. In D61165#1479937 , @rjmccall wrote: > Are you sure these are the right semantics for `nodestroy`? I think there's > a reasonable argument that we should not destroy previous elements of a > `nodestroy` array just because an

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

jfb created this revision. jfb added reviewers: glider, pcc, kcc. Herald added subscribers: cfe-commits, dexonsmith, jkorous. Herald added a project: clang. C guarantees that brace-init with fewer initializers than members in the aggregate will initialize the rest of the aggregate as-if it were

[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

jfb added a comment. In D61165#1479937 , @rjmccall wrote: > Are you sure these are the right semantics for `nodestroy`? I think there's > a reasonable argument that we should not destroy previous elements of a > `nodestroy` array just because an

[PATCH] D63518: BitStream reader: propagate errors

jfb marked an inline comment as done. jfb added inline comments. Comment at: clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp:619 +// FIXME check that the enum is in range. +auto Code = static_cast(MaybeCode.get()); + jfb wrote: > bjope wrote: > >

[PATCH] D63518: BitStream reader: propagate errors

jfb marked 2 inline comments as done. jfb added inline comments. Comment at: clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp:619 +// FIXME check that the enum is in range. +auto Code = static_cast(MaybeCode.get()); + bjope wrote: > This has caused

[PATCH] D64262: Bitstream reader: Fix undefined behavior seen after rL364464

jfb accepted this revision. jfb added a comment. This revision is now accepted and ready to land. Thanks for catching this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64262/new/ https://reviews.llvm.org/D64262

[PATCH] D63518: BitStream reader: propagate errors

jfb marked 2 inline comments as done. jfb added a subscriber: hans. jfb added inline comments. Comment at: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp:205 + return MaybeBitCode.takeError(); +switch (unsigned BitCode = MaybeBitCode.get()) { default: // Default

[PATCH] D63518: BitStream reader: propagate errors

jfb added a comment. I think this is ready to go! I rebased and ran `check-all` for LLVM / clang / clang-tools-extras and everything passes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63518/new/ https://reviews.llvm.org/D63518

[PATCH] D64146: [ConstExprPreter][WIP] Initial patch for the constexpr interpreter

jfb added inline comments. Comment at: clang/lib/AST/ExprVM/Compiler.cpp:85 +/* isArray */ false, +/* isGlobal */ false); + ParamDescriptors.insert({ParamOffset,

[PATCH] D64597: CodeGet: Init 32bit pointers with 0xAAAAAAAA

jfb added a comment. In D64597#1581605 , @pcc wrote: > The problem with `0x` on 32-bit is that it is likely to be a valid > address. > > When I discussed this with JF I proposed a pointer initialization of > `0x` which he agreed to. This

[PATCH] D64597: CodeGet: Init 32bit pointers with 0xFFFFFFFF

jfb added a comment. In D64597#1582944 , @hubert.reinterpretcast wrote: > In D64597#1581605 , @pcc wrote: > > > The problem with `0x` on 32-bit is that it is likely to be a valid > > address. > > > > When

[PATCH] D64666: Allow Clang -Wconversion, -Wimplicit-float-conversion warns about integer type -> floating point type implicit conversion precision loss.

jfb added a comment. I think you want to default-ignore the "may lose precision" warnings, but not the ones that you know always lose precision. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64666/new/ https://reviews.llvm.org/D64666

[PATCH] D64676: Support __seg_fs and __seg_gs on x86

jfb created this revision. Herald added subscribers: cfe-commits, dexonsmith, jkorous. Herald added a project: clang. GCC supports named address spaces macros: https://gcc.gnu.org/onlinedocs/gcc/Named-Address-Spaces.html clang does as well with address spaces:

[PATCH] D64676: Support __seg_fs and __seg_gs on x86

jfb updated this revision to Diff 209645. jfb added a comment. - Fix test order Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64676/new/ https://reviews.llvm.org/D64676 Files: clang/docs/LanguageExtensions.rst clang/lib/Basic/Targets/X86.cpp

[PATCH] D64675: WIP: Disable optimization in emitStoresForConstant

jfb added a comment. That's interesting. If `MemCpyOpt` has picked up the slack and clang can be simpler here, then let's remove clang's "intelligence". I do want to make sure that it handles all the cases this handles though, and does indeed generate great code on both x86-64 and ARM64. I

[PATCH] D64675: WIP: Disable optimization in emitStoresForConstant

jfb added a comment. I would keep single stores because it's pretty silly codegen to copy from a global for that. I would also verify that `-O0` isn't affected too badly by this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64675/new/

[PATCH] D64382: Use getMostFrequentByte to decide if should used memset+stores

jfb added inline comments. Comment at: clang/lib/CodeGen/CGDecl.cpp:865 +/// initializer with equal or fewer than NumStores scalar stores. +static bool canEmitStoresForInitAfterMemset(CodeGenModule , +llvm::Constant *Init,

[PATCH] D64128: [CodeGen] Generate llvm.ptrmask instead of inttoptr(and(ptrtoint, C)) if possible.

jfb added a comment. I'm not sure I understand all the implications, and why that would / wouldn't be valid. Should this be an builtin that can be called from C++ directly? Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2034 + (AllOnes <<

[PATCH] D64164: [clang][ARM] Fix msvc arm{64} builtins to use int on LLP64 systems.

jfb accepted this revision. jfb added inline comments. This revision is now accepted and ready to land. Comment at: test/CodeGen/ms-intrinsics-other.c:212 + return _InterlockedAdd(Addend, Value); +} + Looks like only `cmpxchg` preserves `volatile`? That's not

<    1   2   3   4   5   6   >