[PATCH] D54055: CXXNameMangler::mangleFunctionParam: fix assertion

jfb created this revision. jfb added a reviewer: rjmccall. Herald added subscribers: cfe-commits, dexonsmith. jfb added a comment. I'm not sure this is the right fix because mangling confuses me. It fixes the assertion I'm encountering in my patch, and I don't think I can create a repro without

[PATCH] D54055: CXXNameMangler::mangleFunctionParam: fix assertion

jfb added a comment. I'm not sure this is the right fix because mangling confuses me. It fixes the assertion I'm encountering in my patch, and I don't think I can create a repro without the patch (since I'm asking to mangle a local in a way we don't seem to right now). Repository: rC Clang

[PATCH] D54055: CXXNameMangler::mangleFunctionParam: fix assertion

jfb added a comment. In https://reviews.llvm.org/D54055#1286196, @erik.pilkington wrote: > Can you give an example of some code that triggered this with your patch > applied? Even if it isn't a real reproducer right now, it would help to try > to understand whats happening here. Sure! I have

[PATCH] D54055: CXXNameMangler::mangleFunctionParam: fix assertion

jfb added a comment. In https://reviews.llvm.org/D54055#1286396, @rjmccall wrote: > That sounds more like this use of the mangler isn't manipulating the function > type context correctly. But actually I think the problem is that it's > ridiculous to assume that arbitrary local declarations hav

[PATCH] D54055: CXXNameMangler::mangleFunctionParam: fix assertion

jfb added a comment. In https://reviews.llvm.org/D54055#1286514, @rjmccall wrote: > In https://reviews.llvm.org/D54055#1286397, @jfb wrote: > > > In https://reviews.llvm.org/D54055#1286396, @rjmccall wrote: > > > > > That sounds more like this use of the mangler isn't manipulating the > > > func

[PATCH] D54055: CXXNameMangler::mangleFunctionParam: fix assertion

jfb updated this revision to Diff 173945. jfb added a comment. Herald added subscribers: nhaehnle, jvesely. - As discussed on the review, change the constant generation to name the synthesized constant using '__const.' + function_name + '.' variable_name. The previous code wasn't generally corre

[PATCH] D54055: CXXNameMangler::mangleFunctionParam: fix assertion

jfb added a comment. In https://reviews.llvm.org/D54055#1286523, @rjmccall wrote: > Pfft, if I'm going to be held responsible for my own work, I'm in a lot of > trouble. :) > > Function name + local variable name WFM. Your wish is my command (in this specific instance anyways). Repository:

[PATCH] D54055: CXXNameMangler::mangleFunctionParam: fix assertion

jfb updated this revision to Diff 173946. jfb added a comment. - clang-format Repository: rC Clang https://reviews.llvm.org/D54055 Files: lib/CodeGen/CGDecl.cpp test/CodeGen/decl.c test/CodeGen/dump-struct-builtin.c test/CodeGenCXX/amdgcn-string-literal.cpp test/CodeGenCXX/const-in

[PATCH] D54055: CGDecl::emitStoresForConstant fix synthesized constant's name

jfb updated this revision to Diff 174087. jfb added a comment. Herald added a subscriber: jkorous. - Fix lifetime of the name, Twine bites again. Repository: rC Clang https://reviews.llvm.org/D54055 Files: lib/CodeGen/CGDecl.cpp test/CodeGen/decl.c test/CodeGen/dump-struct-builtin.c

[PATCH] D54055: CGDecl::emitStoresForConstant fix synthesized constant's name

jfb added a comment. Small update to use `std::string` because `Twine` lifetimes are sad. This didn't trigger on any of the tests, but did in a stage2 build. Repository: rC Clang https://reviews.llvm.org/D54055 ___ cfe-commits mailing list cfe-c

[PATCH] D54055: CGDecl::emitStoresForConstant fix synthesized constant's name

jfb updated this revision to Diff 174090. jfb added a comment. - Inline the name creating, save a heap allocation. Repository: rC Clang https://reviews.llvm.org/D54055 Files: lib/CodeGen/CGDecl.cpp test/CodeGen/decl.c test/CodeGen/dump-struct-builtin.c test/CodeGenCXX/amdgcn-string-l

[PATCH] D54055: CGDecl::emitStoresForConstant fix synthesized constant's name

jfb marked an inline comment as done. jfb added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:990-998 + std::string Name = ("__const." + FunctionName(D.getParentFunctionOrMethod()) + + "." + D.getName()) + .str(); + llvm::Glob

[PATCH] D54055: CGDecl::emitStoresForConstant fix synthesized constant's name

This revision was automatically updated to reflect the committed changes. jfb marked an inline comment as done. Closed by commit rL346915: CGDecl::emitStoresForConstant fix synthesized constant's name (authored by jfb, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D54604: Automatic variable initialization

jfb created this revision. jfb added reviewers: kcc, rjmccall, rsmith. Herald added subscribers: dexonsmith, jkorous, JDevlieghere. Add an option to initialize automatic variables with either a pattern or with zeroes. The default is still that automatic variables are uninitialized. Also add attrib

[PATCH] D54604: Automatic variable initialization

jfb added a comment. In https://reviews.llvm.org/D54604#1300544, @t.p.northover wrote: > > This isn't meant to change the semantics of C and C++. > > As I said in the cfe-dev thread, that's exactly what it does. Specifically I > think this is essentially defining a new dialect of C++, which I ha

[PATCH] D54604: Automatic variable initialization

jfb added a comment. In https://reviews.llvm.org/D54604#1301807, @kcc wrote: > Would it be possible to extend test/Sema/uninit-variables.c to verify that > the new option > doesn't break -Wuninitialized? Done. Repository: rC Clang https://reviews.llvm.org/D54604 __

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 174471. jfb added a comment. - Make sure uninit-variables.c doesn't break. Repository: rC Clang https://reviews.llvm.org/D54604 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td include/clang/Basic/LangOptions.def include/clang/Basic

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 177262. jfb marked 10 inline comments as done. jfb added a comment. - Make sure uninit-variables.c doesn't break. - Address Peter's comments, improve tests. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54604/new/ https://reviews

[PATCH] D54604: Automatic variable initialization

jfb added a comment. I've addressed @pcc's comments. I'll now update the patch to mandate a `-Xclang` option for zero-init (otherwise it'll warn and pattern-init instead), and remove documentation for zero-init. I'm also thinking of changing float init to negative NaN with infinite scream paylo

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 177328. jfb added a comment. - Add an ugly option to enable zero init Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54604/new/ https://reviews.llvm.org/D54604 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.t

[PATCH] D54604: Automatic variable initialization

jfb added a comment. I added an option that's required when using `clang` directly (*not* `-cc1`!) If you use `-ftrivial-auto-var-init=zero` without `-enable-trivial-auto-var-init-zero-knowning-it-will-be-removed-from-clang` in `clang` it'll generate a warning and change initialization to patte

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 177331. jfb added a comment. - Update warning-flags.c Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54604/new/ https://reviews.llvm.org/D54604 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td include/clan

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 177341. jfb added a comment. - Fix typo Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54604/new/ https://reviews.llvm.org/D54604 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td include/clang/Basic/Diagno

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 177600. jfb added a comment. - Make sure uninit-variables.c doesn't break. - Address Peter's comments, improve tests. - Add an ugly option to enable zero init - Update warning-flags.c - Fix typo - Use negative NaN with repeated 0xFF payload for all floating-point

[PATCH] D55562: Atomics: support min/max orthogonally

jfb added a comment. What does it do with floating-point inputs? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55562/new/ https://reviews.llvm.org/D55562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

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

jfb added a comment. I think (with test updates) this will be good to go once D58188 lands. Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:1158 + llvm::StructType *STy = dyn_cast(Ty); + if (STy && (STy == Loc.getElementType()) && + shou

[PATCH] D57898: CodeGen: Fix PR40605

jfb added a comment. Can you change the title to be more descriptive? Small test fixes, otherwise this LGTM, will check box after update. Comment at: clang/test/CodeGenCXX/auto-var-init.cpp:123 +// PATTERN-NOT-O1: @__const.test_int1_custom.custom +// ZERO-NOT-O1: @__const.test

[PATCH] D57898: CodeGen: Fix PR40605 by splitting constant struct initializers

jfb accepted this revision. jfb added a comment. This revision is now accepted and ready to land. LGTM, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57898/new/ https://reviews.llvm.org/D57898 ___ cfe-commits mailing list cfe-commit

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

jfb created this revision. jfb added reviewers: glider, pcc, kcc, rjmccall. Herald added subscribers: cfe-commits, jdoerfert, dexonsmith, jkorous. Herald added a project: clang. Following up with r355181, initialize small arrays as well. Repository: rC Clang https://reviews.llvm.org/D58885 F

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

jfb updated this revision to Diff 189085. jfb marked an inline comment as done. jfb added a comment. - typo Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58885/new/ https://reviews.llvm.org/D58885 Files: lib/CodeGen/CGDecl.cpp test/CodeGenCXX/auto-var-init.c

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

jfb added a comment. I'll do a few size diffs to double-check that this also pays off. @glider can you also check that it doesn't regress what you've been looking at? Comment at: test/CodeGenCXX/auto-var-init.cpp:1133 // PATTERN-O1: bitcast -// PATTERN-O1: call void @llvm.mem

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

jfb marked 2 inline comments as done. jfb added inline comments. Comment at: test/CodeGenCXX/auto-var-init.cpp:1025 // PATTERN-LABEL: @test_intptr4_uninit() -// PATTERN: call void @llvm.memcpy{{.*}} @__const.test_intptr4_uninit.uninit -// ZERO-LABEL: @test_intptr4_uninit() -// Z

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

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

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

jfb marked 3 inline comments as done. jfb added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1206 + bool canDoSingleStore = Ty->isIntOrIntVectorTy() || + Ty->isPtrOrPtrVectorTy() || Ty->isFPOrFPVectorTy(); + if (canDoSingleStore) { --

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

jfb marked an inline comment as done. jfb added a comment. Comparing clang stage2 in release mode, with an without this change, we see a 408 byte size difference, which is ~nothing. Here's details, nothing surprising: $ /s/bloaty/bloaty -d sections /s/llvm1/llvm/stage2/bin/clang-9 -- /s/llvm

[PATCH] D59032: Passthrough compiler launcher

jfb created this revision. jfb added reviewers: ddunbar, dexonsmith. Herald added subscribers: cfe-commits, jdoerfert, jkorous, mgorny. Herald added a project: clang. Not having this seems like an oversight, and makes stage2 builds odd. Repository: rC Clang https://reviews.llvm.org/D59032 Fi

[PATCH] D59032: Passthrough compiler launcher

This revision was automatically updated to reflect the committed changes. Closed by commit rC355547: Passthrough compiler launcher (authored by jfb, committed by ). Changed prior to commit: https://reviews.llvm.org/D59032?vs=189537&id=189563#toc Repository: rC Clang CHANGES SINCE LAST ACTIO

[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 ACTION

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

jfb added a comment. I don't think this is quite right: CX16 is literally "I have cmpxchg16b". In clang/lib/Basic/Targets/X86.h we do: void setMaxAtomicWidth() override { if (hasFeature("cx16")) MaxAtomicInlineWidth = 128; } Your change makes it inc

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

jfb added a comment. In D59287#1427485 , @craig.topper wrote: > Isn’t that setMaxAtomicWidth in the x86-64 derived class? Right you are! > As far as preventing “cx16” from being set in 32-bit mode, we’ll need to > check the behavior of CPUID in 32-bit

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

jfb requested changes to this revision. jfb added a comment. This revision now requires changes to proceed. I think you also want to test C++ `std::atomic` as well as regular `volatile`. Comment at: test/Sema/atomic-expr-stmt.c:7 + //CHECK: %atomic-load = load atomic i32, i32*

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

jfb added a comment. In D59287#1427654 , @craig.topper wrote: > Most if not all of the test cases in test/CodeGen/X86/atomic128.ll fail with > a fatal error if you run it in 32-bit mode with -mattr=+cx16 Looks like the > backend is also bad at checking

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

jfb added a comment. Thinking some more, this is really a silly gotcha in code. We should probably follow what we're about to do with wg21.link/P1152 and ban chaining. Statement expressions should therefore not be allowed to return `_Atomic`, `std::atomic`, nor

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

jfb added a subscriber: jwakely. jfb added a comment. From an offline discussion, I'm told that @jwakely uses this in GCC headers and expects some semantics from it? Jonathan, why?!?! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59307/new/ https://reviews.llvm.

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

jfb added a comment. In D59287#1427945 , @craig.topper wrote: > Is this ok with the backend fixed? This is definitely better. > Or do you want me factor this into HasCX16 which I think is only used by the > defineMacro and the return for hasFeature("c

[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 https://bugs.llvm.org/show_bug.cgi?id=4

[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/ https://reviews.llv

[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? Repository

[PATCH] D59254: [RFC] Implementation of Clang randstruct

jfb requested changes to this revision. jfb added a comment. This revision now requires changes to proceed. I find it easier to understand the code by looking at the tests. When you add tests, please make sure you test for: - Alignment attribute - Packed attribute - No unique address attribute -

[PATCH] D59446: CodeGen: Preserve packed attribute in constStructWithPadding.

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

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

jfb created this revision. jfb added reviewers: dexonsmith, erik.pilkington. Herald added subscribers: cfe-commits, jdoerfert, jkorous. Herald added a project: clang. jfb added a comment. It seems weird how both do pretty similar things and e.g. duplicate `getParamDecl`. The repro I have is 20M,

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

jfb added a comment. It seems weird how both do pretty similar things and e.g. duplicate `getParamDecl`. The repro I have is 20M, creduce chokes on it, and manually reducing failed to repro... ☹️ Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59523/new/ https:/

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

jfb added a comment. In D59523#1434121 , @erik.pilkington wrote: > I don't understand this code at all, but what about `BlockDecl`? Agreed that it has `getParamDecl` as well, and the context would fit. It just seems weird how stuff is repeated, Iw as h

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

jfb added a comment. Why was it reverted? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D28213/new/ https://reviews.llvm.org/D28213 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin

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

jfb added a comment. In D28213#1435485 , @efriedma wrote: > It looks like it was reverted because it was breaking i386 BSD, where > __GCC_ATOMIC_LLONG_LOCK_FREE is in fact supposed to be "1" (because cmpxchg8b > isn't always available). Do we care abou

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

jfb updated this revision to Diff 191416. jfb added a comment. - Use suggested format. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59523/new/ https://reviews.llvm.org/D59523 Files: lib/Analysis/ThreadSafetyCommon.cpp Index: lib/Analysis/ThreadSafetyCommon.

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

jfb added a comment. I reduced the code I had to this: struct __attribute__((capability("mutex"))) MyLock { void lock() __attribute__((acquire_capability())) {} void unlock() __attribute__((release_capability())) {} }; template struct __attribute__((scoped_lockable)) Locker {

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

jfb added a comment. Actually I'm wrong, this repros properly, will send an update with test. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59523/new/ https://reviews.llvm.org/D59523 ___ cfe-commits mailing list cfe-

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

jfb updated this revision to Diff 191417. jfb added a comment. - Add test Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59523/new/ https://reviews.llvm.org/D59523 Files: lib/Analysis/ThreadSafetyCommon.cpp test/SemaObjCXX/no-crash-thread-safety-analysis.mm

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

jfb updated this revision to Diff 191580. jfb marked 3 inline comments as done. jfb added a comment. - Simlify tests, share code Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59523/new/ https://reviews.llvm.org/D59523 Files: lib/Analysis/ThreadSafetyCommon.cpp

[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 + aaronpuchert wrote: > Test is fine for me, but I

[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: > > j

[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: > > aaron.ball

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

jfb added inline comments. Comment at: lib/Analysis/ThreadSafetyCommon.cpp:280 unsigned I = PV->getFunctionScopeIndex(); - -if (Ctx && Ctx->FunArgs && FD == Ctx->AttrDecl->getCanonicalDecl()) { - // Substitute call arguments for references to function parameters -

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

jfb updated this revision to Diff 191939. jfb marked 13 inline comments as done. jfb added a comment. - Almost Never Auto. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59523/new/ https://reviews.llvm.org/D59523 Files: lib/Analysis/ThreadSafetyCommon.cpp tes

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

jfb added a comment. In D59523#1440232 , @aaronpuchert wrote: > > It's less about the regressions and more about the kind of regressions > > we're testing against. > > But the test also verifies that no diagnostics are omitted (`// > expected-no-diagnos

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

jfb added a comment. In D59523#1440263 , @aaron.ballman wrote: > In D59523#1440238 , @jfb wrote: > > > In D59523#1440232 , @aaronpuchert > > wrote: > > > > > > It's less ab

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

jfb updated this revision to Diff 191968. jfb added a comment. - No verify, no expected. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59523/new/ https://reviews.llvm.org/D59523 Files: lib/Analysis/ThreadSafetyCommon.cpp test/SemaObjCXX/no-crash-thread-safet

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL356940: Thread Safety: also look at ObjC methods (authored by jfb, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION

[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 h

[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/ https://revi

[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 wondering

[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: lib/C

[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] 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&id=194787#toc Repository: rC Clang CHANGES SINC

[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 cfe-commits@lists.llvm.o

[PATCH] D61106: [analyzer][UninitializedObjectChecker] PR41590: Regard _Atomic types as primitive

jfb added a comment. Have you checked what the C and C++ standards say about `_Atomic` / `std::atomic` initialization, and how they should actually be initialized by developers? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61106/new/ https://reviews.llvm.org/D

[PATCH] D55229: [COFF] Statically link certain runtime library functions

jfb added a comment. Looks like this might have broken bots: /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/CodeGenCXX/runtime-dllstorage.cpp:121:26: error: CHECK-IA-DAG: expected string not found in input // CHECK-DYNAMIC-IA-DAG: declare dllimpo

[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 element

[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 st

[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 argu

[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 element

[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: don't initialize aggregate padding of all aggregates (authored by jfb, committed by ). Changed prior to commit: https://reviews.llvm.org/D61280?vs=197188&id=197476#toc Repos

[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 m

[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 http

[PATCH] D51752: NFC: deduplicate isRepeatedBytePattern from clang to LLVM's isBytewiseValue

This revision was automatically updated to reflect the committed changes. Closed by commit rL342734: NFC: deduplicate isRepeatedBytePattern from clang to LLVM's isBytewiseValue (authored by jfb, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.o

[PATCH] D52339: Support enums with a fixed underlying type in all language modes

jfb added a comment. I think we should consider proposing this to the C committee. @aaron.ballman I can help Erik write the paper, would you be able to present it? Too tight for the upcoming meeting, but I'm guessing we have *plenty* of time for ++C17. Repository: rC Clang https://reviews.l

[PATCH] D52339: Support enums with a fixed underlying type in all language modes

jfb added a comment. In https://reviews.llvm.org/D52339#1242103, @aaron.ballman wrote: > In https://reviews.llvm.org/D52339#1242086, @jfb wrote: > > > I think we should consider proposing this to the C committee. > > @aaron.ballman I can help Erik write the paper, would you be able to > > prese

[PATCH] D52892: [Clang-tidy] readability check to convert numerical constants to std::numeric_limits

jfb added a comment. Can you also catch casts such as `(unsigned)-1` ? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52892 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D53001: [Driver] check for exit code from SIGPIPE

jfb added a comment. What's the return code of the driver when the pipe is broken that way? Comment at: lib/Driver/Driver.cpp:1406 +// for SIGPIPE. Do not print diagnostics for this case. +if (Res == 71) + continue; Ditto on magical number in a hea

[PATCH] D53001: [Driver] check for exit code from SIGPIPE

jfb added inline comments. Comment at: lib/Driver/Driver.cpp:1406 +// for SIGPIPE. Do not print diagnostics for this case. +if (Res == 71) + continue; nickdesaulniers wrote: > jfb wrote: > > jfb wrote: > > > Ditto on magical number in a header. > > I

[PATCH] D53001: [Driver] check for exit code from SIGPIPE

jfb added a comment. Just to be sure: what's the exit code from the driver? If we don't diagnose I'm fine with it... but the exit code still needs to reflect the failure! Repository: rC Clang https://reviews.llvm.org/D53001 ___ cfe-commits maili

[PATCH] D53001: [Driver] check for exit code from SIGPIPE

jfb accepted this revision. jfb added a comment. This revision is now accepted and ready to land. Digging through `tools/driver/driver.cpp` this seems to be the right thing. Maybe leave it open for a bit so others can chime in (if say they have other drivers or whatever?). Thanks for fixing! R

[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 str

[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 zero-siz

<    1   2   3   4   5   6   >