[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

jyknight added a comment. It looks to me from GCC that constraint 'p' is intended to be used internally, not for inline-asm. I question whether the kernel should be using it, and whether we should even implement it at all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

jyknight added a comment. > ‘p’ in the constraint must be accompanied by address_operand as the predicate > in the match_operand. This predicate interprets the mode specified in the > match_operand as the mode of the memory reference for which the address would > be valid. How do you do that

[PATCH] D144889: [C2x] Support in freestanding

jyknight added a comment. In D144889#4156974 , @rsmith wrote: > Likely because of GCC's perspective on this, the set of C headers provided by > GCC, Clang, ICC, etc. has included the complete list of freestanding headers > and more or less no others,

[PATCH] D122258: [MC] Omit DWARF unwind info if compact unwind is present where eligible

jyknight added a comment. > no-compact-unwind is particularly useful for newer x86_64 platforms: we don't > want to omit DWARF unwind for x86_64 in general due to possible backwards > compat issues, but we should make it possible for people to opt into this > behavior if they are only

[PATCH] D144889: [C2x] Support in freestanding

jyknight added a comment. > On the other hand, I think a not-insignificant number of users are interested > in freestanding environments for one-off/fun/experimental projects where ease > of access is more important than performance characteristics -- think: users > who are playing around with

[PATCH] D145416: [clang] model 'p' inline asm constraint as reading memory

jyknight requested changes to this revision. jyknight added a comment. This revision now requires changes to proceed. I believe this is abandoned, correct? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145416/new/ https://reviews.llvm.org/D145416

[PATCH] D123085: CGExprCXX: emit allocptr attributes for operator delete

jyknight added inline comments. Comment at: clang/lib/CodeGen/CGExprCXX.cpp:1333 +CalleeDecl->getOverloadedOperator() == OO_Delete) { + CallOrInvoke->addParamAttr(0, llvm::Attribute::AllocatedPointer); +} Perhaps we should add this attribute to

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

jyknight added a comment. In D148700#4418767 , @rsandifo-arm wrote: > Hi @jyknight , @rsmith > > Do you have any more thoughts on the above? Quick version is: > > 1. Is it OK to have `[[…]]` attributes in the `arm` namespace that affect > semantics?

[PATCH] D152401: CGExprCXX: emit family and allockind attributes

jyknight added a comment. > As a result, clang/llvm should correctly > optimize custom allocators like this without needing specialized > knowledge going forward. Not exactly: we still need knowledge of this in //clang// (`isReplaceableGlobalAllocationFunction` was taught that the variant with

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

jyknight added inline comments. Comment at: clang/lib/Sema/JumpDiagnostics.cpp:658 +if (auto *G = dyn_cast(Jump)) { + for (AddrLabelExpr *L : G->labels()) { efriedma wrote: > nickdesaulniers wrote: > > efriedma wrote: > > > nickdesaulniers wrote: > >

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

jyknight added a comment. In D150226#4400782 , @rupprecht wrote: > As a general question/feature request: is there a way to have specific > warnings apply even for system headers? It would be nice if I could check > what breaks when by adding

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

jyknight added a comment. > This makes ``CXX11`` and ``C2x`` spellings > unsuitable for attributes that affect the type system, that change the > binary interface of the code, or that have other similar semantic meaning. Yes, standard attributes aren't supposed to be used for things which affect

[PATCH] D141918: WIP: [Clang] Emit 'unwindabort' when applicable.

jyknight updated this revision to Diff 546613. jyknight edited the summary of this revision. jyknight added a comment. Rebase patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141918/new/ https://reviews.llvm.org/D141918 Files:

[PATCH] D156247: [Clang] Add a warning on uses of coroutine keywords

jyknight added a comment. As I posted on the RFC thread, I think we have a viable alternative solution to address the original motiving use-case. One might potentially still make a case for implementing the `-fno-coroutines` flag for GCC compatibility, but given the concerns raised -- and that

[PATCH] D155387: [Clang] Fix member lookup so that we don't ignore ambiguous lookups in some cases

jyknight added a comment. In D155387#4557834 , @hctim wrote: > I found an issue with building Android using this patch. I've reduced it down > to the following problem where the evaluation of the `std::visit` is believed > to be non-exhaustive, but it

[PATCH] D141918: WIP: [Clang] Emit 'unwindabort' when applicable.

jyknight added a comment. In D141918#4566838 , @smeenai wrote: > $ clang -std=c++20 -O2 -c crash.cpp > cannot use musttail with unwindabort Thanks for the report. We were missing a check of `CI.isUnwindAbort()` in CoroSplit.cpp's `shouldBeMustTail()`

[PATCH] D156286: [docs] Bump minimum GCC version to 7.5

jyknight added a comment. Given that folks have successfully tested with GCC 7.4, and the lateness of the change in the release process for LLVM 17, I think it'd be better to require GCC 7.4 (the earliest that actually works), instead of increasing the requirement to 7.5. Repository: rG

[PATCH] D121629: clang: also check alloc_alignment claims in return

jyknight added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:3684 + const AllocAlignAttr *AA = CurCodeDecl->getAttr(); + if (SanOpts.has(SanitizerKind::Alignment) && AA) { It may be nice to also verify the alignment required by an AssumeAlignAttr

[PATCH] D144999: [RFC][MC][MachO]Only emits compact-unwind format for "canonical" personality symbols. For the rest, use DWARFs.

jyknight added a comment. I wonder if we actually need to define a clang frontend flag for this; I suspect nobody will ever want to specify it, since the only non-canonical personality clang will ever generate that changes behavior is the pure-C destructor-only personality,

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

jyknight added a comment. When looking for errors in existing codebases, don't forget that this diagnostic is currently suppressed by default in system headers. So this patch is moving from "no diagnostics for code in system headers" to "unconditional hard error in system headers". Just

<    1   2   3   4   5