[PATCH] D51084: Implement -Watomic-implicit-seq-cst

jfb added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10974 + if (E->IgnoreParenImpCasts()->getType()->isAtomicType()) +return; CheckImplicitConversion(S, E->IgnoreParenImpCasts(), S.Context.BoolTy, CC); rjmccall wrote: > Can you explain this

[PATCH] D51752: NFC: move isRepeatedBytePattern from clang to Constant

jfb updated this revision to Diff 164481. jfb added a comment. - Use isBytewiseValue instead. Repository: rC Clang https://reviews.llvm.org/D51752 Files: lib/CodeGen/CGDecl.cpp Index: lib/CodeGen/CGDecl.cpp === ---

[PATCH] D51752: NFC: move isRepeatedBytePattern from clang to Constant

jfb added a comment. In https://reviews.llvm.org/D51752#1226474, @MatzeB wrote: > In https://reviews.llvm.org/D51752#1226462, @MatzeB wrote: > > > - I assume the review lacks the changes on the llvm side? > > > Oops missed the link to the llvm changes in the description... At least in > theory

[PATCH] D51752: NFC: move isRepeatedBytePattern from clang to Constant

jfb created this revision. Herald added subscribers: cfe-commits, dexonsmith. This code was in CGDecl.cpp and really belongs to Constant. This will allow me to use it in a subsequent patch. LLVM part of this patch: https://reviews.llvm.org/D51751 Repository: rC Clang

[PATCH] D51752: NFC: move isRepeatedBytePattern from clang to Constant

jfb added a comment. In https://reviews.llvm.org/D51752#1226462, @MatzeB wrote: > - I'm not a good person to review clang changes. > - I assume the review lacks the changes on the llvm side? > - On the LLVM side this feels like something for `Analysis/ValueTracking.h` > in fact I already see

[PATCH] D51752: NFC: move isRepeatedBytePattern from clang to Constant

jfb added a comment. In https://reviews.llvm.org/D51752#1226465, @jfb wrote: > In https://reviews.llvm.org/D51752#1226462, @MatzeB wrote: > > > - I'm not a good person to review clang changes. > > - I assume the review lacks the changes on the llvm side? > > - On the LLVM side this feels like

[PATCH] D51084: Implement -Watomic-implicit-seq-cst

This revision was automatically updated to reflect the committed changes. Closed by commit rC341860: Implement -Watomic-implicit-seq-cst (authored by jfb, committed by ). Changed prior to commit: https://reviews.llvm.org/D51084?vs=164235=164746#toc Repository: rC Clang

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

jfb added a comment. Will wait on https://reviews.llvm.org/D51751 before committing, otherwise clang tests will start failing (because current `isRepeatedBytePattern` isn't capable enough). Repository: rC Clang https://reviews.llvm.org/D51752

[PATCH] D48852: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %tu/%td on Darwin

jfb accepted this revision. jfb added a comment. This revision is now accepted and ready to land. LGTM after a few questions. Comment at: include/clang/Analysis/Analyses/FormatString.h:265 + enum class TypeKind { Unspecified, SizeT, PtrdiffT }; + TypeKind TK =

[PATCH] D49209: CodeGen: specify alignment for automatic variable initialization

jfb added a comment. I updated the patch to use `Address`, and also use `inbounds`. This was a bit tricky because of the GEPs. I also updated tests which run into this codegen. Only the following tests actually hit this code, and not all check these stores: Clang ::

[PATCH] D49209: CodeGen: specify alignment for automatic variable initialization

jfb updated this revision to Diff 155287. jfb added a comment. - Fix silly naming and lookup. Repository: rC Clang https://reviews.llvm.org/D49209 Files: lib/CodeGen/CGBuilder.h lib/CodeGen/CGDecl.cpp test/CodeGen/init.c test/CodeGenOpenCL/partial_initializer.cl Index:

[PATCH] D49209: CodeGen: specify alignment for automatic variable initialization

jfb updated this revision to Diff 155285. jfb marked an inline comment as done. jfb added a comment. - Use Address as suggested in review. Repository: rC Clang https://reviews.llvm.org/D49209 Files: lib/CodeGen/CGBuilder.h lib/CodeGen/CGDecl.cpp test/CodeGen/init.c

[PATCH] D49209: CodeGen: specify alignment for automatic variable initialization

jfb created this revision. Herald added subscribers: cfe-commits, dexonsmith. Automatic variable initialization was generating default-aligned stores (which are deprecated) instead of using the known alignment from the alloca. Repository: rC Clang https://reviews.llvm.org/D49209 Files:

[PATCH] D49209: CodeGen: specify alignment for automatic variable initialization

jfb updated this revision to Diff 155424. jfb added a comment. - Simplify CreateStore. Repository: rC Clang https://reviews.llvm.org/D49209 Files: lib/CodeGen/CGBuilder.h lib/CodeGen/CGDecl.cpp test/CodeGen/init.c test/CodeGenOpenCL/partial_initializer.cl Index:

[PATCH] D49209: CodeGen: specify alignment for automatic variable initialization

jfb marked an inline comment as done. jfb added inline comments. Comment at: lib/CodeGen/CGBuilder.h:260 +CharUnits::fromQuantity(Offset.getSExtValue(; + } + efriedma wrote: > Not sure about the new helper. We already have

[PATCH] D49209: CodeGen: specify alignment for automatic variable initialization

This revision was automatically updated to reflect the committed changes. Closed by commit rL337041: CodeGen: specify alignment + inbounds for automatic variable initialization (authored by jfb, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D53154: [CodeGen] Handle extern references to OBJC_CLASS_$_*

jfb added a comment. Overall this seems fine, but I'm no ObjC expert either so I'll defer to @rjmccall Comment at: clang/lib/CodeGen/CGObjCMac.cpp:7193 + if (!GV || GV->getType() != ObjCTypes.ClassnfABITy->getPointerTo()) { +auto *NewGV = new

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

[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

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

[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

[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

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

[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

[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 constants name (authored by jfb, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[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

[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

[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(); +

[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

[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div

jfb added a comment. Can you add tests with arrays? The error message doesn't really say what to do instead. What's wrong? What's correct instead? In C++17 and later we should suggest using `std::size` for some cases instead. https://reviews.llvm.org/D52949

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

[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

[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

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

[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 LLVMs isBytewiseValue (authored by jfb, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

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

[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

[PATCH] D54604: Automatic variable initialization

jfb marked an inline comment as done. jfb added inline comments. Comment at: cfe/trunk/include/clang/Driver/Options.td:1657 + " | pattern">, Values<"uninitialized,pattern">; +def enable_trivial_var_init_zero : Joined<["-"],

[PATCH] D56760: Add a new builtin: __builtin_dynamic_object_size

jfb added a comment. I pointed out this review to Jonathan Wakely, who posted it to the GCC list . Jakub replied with: > The current modes (0-3) certainly must not be changed and must return a > constant, that is what huge amounts of code in

[PATCH] D56760: Add a new builtin: __builtin_dynamic_object_size

jfb added a comment. In D56760#1368216 , @rsmith wrote: > In D56760#1368054 , @erik.pilkington > wrote: > > > So it seems like the GCC people want to keep `__builtin_object_size` static. > > > I don't see any

[PATCH] D54604: Automatic variable initialization

jfb added a comment. In D54604#1327893 , @rsmith wrote: > For the record: I'm OK with this direction. (I somewhat prefer removing the > `-enable-long-wordy-thing` option and instead automatically disabling the > `zero` option for clang release builds,

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 178129. jfb marked 3 inline comments as done. jfb added a comment. - Don't document 'zero'. - Make drv_trivial_auto_var_init_zero_disabled an error instead of a warning. - Change the parameter: we only have [[clang::uninitialized]] now. Repository: rC Clang

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 178566. jfb added a comment. - Update attribute test. 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

[PATCH] D54604: Automatic variable initialization

This revision was automatically updated to reflect the committed changes. Closed by commit rL349442: Automatic variable initialization (authored by jfb, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D54604?vs=178566=178590#toc

[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

[PATCH] D54604: Automatic variable initialization

jfb added a comment. In D54604#1331711 , @pcc wrote: > Mostly looking good, a few style nits. Thanks! I think I addressed all of your comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54604/new/

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 178301. jfb marked 6 inline comments as done. jfb added a comment. - Address @pcc's comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54604/new/ https://reviews.llvm.org/D54604 Files: include/clang/Basic/Attr.td

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 178304. jfb added a comment. - Rebase 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

[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

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

[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

[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

[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

[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

[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

[PATCH] D55895: NFC: simplify Darwin environment handling

jfb created this revision. jfb added reviewers: dexonsmith, arphaman. Herald added subscribers: cfe-commits, jkorous. The previous code detected conflicts through copy-pasta, this versions uses a 'loop'. Repository: rC Clang https://reviews.llvm.org/D55895 Files:

[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 (`// >

[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

[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

[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

[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] 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()) && +

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

[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

[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 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() -//

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

[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

[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=189563#toc Repository: rC Clang CHANGES SINCE LAST ACTION

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

[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

[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

[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

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

[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

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

[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

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

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

[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

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

[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

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

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

[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

[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

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

[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

<    1   2   3   4   5   6   >