[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

jyknight added a comment. This patch as it stands is harmless, since as it only defines an internal interface, which is unused. So in that sense, it's perfectly fine to commit even with the remaining unresolved questions about the correct values to return. However, unless we're going to actuall

[PATCH] D60748: Adds an option "malign-pass-aggregate" to make the alignment of the struct and union parameters compatible with the default gcc

jyknight added a comment. Since the ABI this is trying to match is not documented literally anywhere, I think we need to have some confidence that what this implements is actually the same as what GCC does. While I wrote up what I think the algorithm is, without some sort of script to allow tes

[PATCH] D67983: [ObjC] Diagnose implicit type coercion from ObjC 'Class' to object pointer types.

jyknight added a comment. In D67983#1863019 , @arphaman wrote: > @jyknight @rjmccall I'm not sure this change is 100% fine. For example, the > following code no longer compiles with ARC: > > @protocol Delegate > @end > > @interface X > > @end

[PATCH] D67983: [ObjC] Diagnose implicit type coercion from ObjC 'Class' to object pointer types.

jyknight added a comment. >> Your error looks correct to me -- "self" in a classmethod is not an >> instance, but the class itself. And while instances of X implement >> "Delegate", the Class does not. > > Got it, thanks! We might need to add a flag to allow the old behavior > temporarily to a

[PATCH] D69868: Allow "callbr" to return non-void values

jyknight added a comment. The idea of moving the copies to a new MachineBasicBlock seems a reasonable solution. That said, it does mean there will be allocatable physical registers which are live-in to the following block, which is generally not allowed. As far as I can tell, I _think_ that sho

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

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

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

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

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

jyknight added a comment. Also, I note the doc says it's useful for `for “load address” and “push address” instructions` (note, "load address" means e.g. x86 "lea" instruction) -- which should NOT be dependent upon the value stored in the memory. The x86 backend actually uses a "Ts" constraint

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

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

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

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

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

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

[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] D139652: Add the thread sanitizer support for X86_64 WatchOS simulators

jyknight accepted this revision. jyknight added a comment. This revision is now accepted and ready to land. Thanks for the change. The description is a bit misleading, this would be better: Allow TSan in clang driver for X86_64 WatchOS simulator. It was already functional, and Apple's dow

[PATCH] D24688: Introduce "hosted" module flag.

jyknight added a comment. Since this requires everyone generating llvm IR to add this module attribute for optimal codegen, it should be documented in release notes and the LangRef, I think. I mean: it's unfortunate that it's needed at all, but at the very least it should be possible for peopl

[PATCH] D27387: [libc++] Add a key function for bad_function_call

jyknight added a comment. Since this is only adding a new export to libc++, not changing/breaking existing ones, I don't think it should be grouped with the ABI-breaking changes in v2. The usual promise is that upgrading libc++.so.1 will not break apps compiled against an older set of headers,

[PATCH] D30427: Fix whitespace before token-paste of an argument.

jyknight added a comment. Ping. https://reviews.llvm.org/D30427 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31022: Implement P0298R3: `std::byte`

jyknight added a comment. I believe this needs compiler support, too, in order to treat namespace std { enum class byte : unsigned char {}; } as directly having tbaa type "omnipotent char" instead of a subtype. That is, given: void foo(char* x, int *y) { x[1] = char(y[0] & 0xff); x

[PATCH] D31022: Implement P0298R3: `std::byte`

jyknight added a comment. Ping. I don't think this got resolved, and I really wouldn't like to see released in this state...can you either revert this from the library, or implement the compiler support, before the 5.0 release branch? In https://reviews.llvm.org/D31022#716702, @jyknight wrote:

[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

jyknight added inline comments. Comment at: lib/Driver/ToolChains/Linux.cpp:755 +!Version.isOlderThan(4, 8, 0)) { + // For gcc >= 4.8.x, clang will preinclude + // -ffreestanding suppresses this behavior. I don't see why it makes any sense to c

[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

jyknight added inline comments. Comment at: lib/Driver/ToolChains/Linux.cpp:755 +!Version.isOlderThan(4, 8, 0)) { + // For gcc >= 4.8.x, clang will preinclude + // -ffreestanding suppresses this behavior. jyknight wrote: > I don't see why it ma

[PATCH] D34158: For standards compatibility, preinclude if the file is available

jyknight added a comment. This version is still disabling upon -nostdinc, which doesn't make sense to me. You didn't remove that, nor respond explaining why you think it does make sense? https://reviews.llvm.org/D34158 ___ cfe-commits mailing list

[PATCH] D34294: Rework libcxx strerror_r handling.

This revision was automatically updated to reflect the committed changes. Closed by commit rL308528: Rework libcxx strerror_r handling. (authored by jyknight). Repository: rL LLVM https://reviews.llvm.org/D34294 Files: libcxx/trunk/src/system_error.cpp Index: libcxx/trunk/src/system_error

[PATCH] D42742: [clangd] Use pthread instead of thread_local to support more runtimes.

jyknight added a comment. Is there some way to figure out what's going on in clang-x86_64-linux-selfhost-modules? I believe there should be no linux builders which are missing this function -- it was added to libgcc in 4.8, and we don't support older versions, so I think missing this function

[PATCH] D30427: Fix whitespace before token-paste of an argument.

jyknight created this revision. The whitespace should come from the argument name in the macro expansion, rather than from the token passed to the macro (same as it does when not pasting). Added a new test case for the change in behavior to stringize_space.c. FileCheck'ized macro_paste_commaext.

[PATCH] D30427: Fix whitespace before token-paste of an argument.

jyknight added a comment. Ping https://reviews.llvm.org/D30427 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30427: Fix whitespace before token-paste of an argument.

This revision was automatically updated to reflect the committed changes. Closed by commit rL302195: Fix whitespace before token-paste of an argument. (authored by jyknight). Changed prior to commit: https://reviews.llvm.org/D30427?vs=89918&id=97881#toc Repository: rL LLVM https://reviews.l

[PATCH] D33020: [Myriad] Pass -Xclang and -mllvm flags to moviCompile

jyknight accepted this revision. jyknight added a comment. This revision is now accepted and ready to land. Will submit for you since you don't have an svn account. https://reviews.llvm.org/D33020 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D33020: [Myriad] Pass -Xclang and -mllvm flags to moviCompile

This revision was automatically updated to reflect the committed changes. Closed by commit rL302738: [Myriad] Pass -Xclang and -mllvm flags to moviCompile (authored by jyknight). Changed prior to commit: https://reviews.llvm.org/D33020?vs=98369&id=98538#toc Repository: rL LLVM https://revie

[PATCH] D29117: SPARC: allow usage of floating-point registers in inline ASM

This revision was automatically updated to reflect the committed changes. Closed by commit rL302913: [SPARC] Support 'f' and 'e' inline asm constraints. (authored by jyknight). Changed prior to commit: https://reviews.llvm.org/D29117?vs=85708&id=98782#toc Repository: rL LLVM https://reviews

[PATCH] D117304: [clang][dataflow] Remove TestingSupport's dependency on gtest

jyknight added a comment. Herald added a subscriber: steakhal. > Users outside of the clang repo may use different googletest versions. I don't understand what that means. Why does it matter what version outside users are using -- these are clang unit-tests, not a public API, right? Repositor

[PATCH] D117238: [C2x] Add BITINT_MAXWIDTH support

jyknight accepted this revision. jyknight added a comment. No additional comments, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117238/new/ https://reviews.llvm.org/D117238 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D118297: [clang] add Diag -Wasm-volatile for implied volatile asm stmts

jyknight added a comment. I agree that's the expected semantics. I think those semantics are unfortunate, but they're not gonna change. IMO it would've been better if you had to opt-in to "no side effects" via `__attribute__((const))` or so. But I wonder why you think we should be discouraging

[PATCH] D116544: [Clang] Introduce Clang Linker Wrapper Tool

jyknight added a comment. "clang-linker-wrapper" seems like a very generic name for a command which is OpenMP offloading specific? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116544/new/ https://reviews.llvm.org/D116544

[PATCH] D118804: Revert "[Clang] Propagate guaranteed alignment for malloc and others"

jyknight created this revision. jyknight added reviewers: aaron.ballman, rjmccall, jdoerfert, xbolva00. jyknight requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The above change assumed that malloc (and friends) would always allocate memory

[PATCH] D118804: Revert "[Clang] Propagate guaranteed alignment for malloc and others"

jyknight updated this revision to Diff 405315. jyknight added a comment. (fix git mishap: neglected to add a file to original change after conflict resolution) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118804/new/ https://reviews.llvm.org/D118

[PATCH] D118804: Revert "[Clang] Propagate guaranteed alignment for malloc and others"

jyknight added a comment. In D118804#3290803 , @xbolva00 wrote: > GCC also does same assumptions Looking at GCC: GCC only assumes 4-byte alignment on i386, and 8-byte alignment on x86-64, which is why it hasn't actively broken users, unlike the clang c

[PATCH] D118804: Revert "[Clang] Propagate guaranteed alignment for malloc and others"

jyknight added a comment. In D118804#3290874 , @collares wrote: > It is worth noting that GCC started assuming 16-byte alignment for small > objects in 2016, before N2293 was written and accepted into C2x; see > https://gcc.gnu.org/bugzilla/show_bug.cgi

[PATCH] D118804: Revert "[Clang] Propagate guaranteed alignment for malloc and others"

jyknight added a subscriber: collares. jyknight added a comment. In D118804#3290874 , @collares wrote: > might be worth filing a GCC bug as well Yes, a bug report for GCC should be opened as well. @collares do you want to take that? In D118804#3290937

[PATCH] D118804: Revert "[Clang] Propagate guaranteed alignment for malloc and others"

jyknight added a comment. > For C++, I confess I have some problems interpreting this sentence: > > (https://eel.is/c++draft/cpp.predefined) > >> `__STDCPP_­DEFAULT_­NEW_­ALIGNMENT__`: An integer literal of type >> std::size_t whose value is the alignment guaranteed by a call to operator >> new

[PATCH] D120159: [Clang] Implement __builtin_source_location.

jyknight created this revision. jyknight added reviewers: aaron.ballman, rsmith. Herald added subscribers: dexonsmith, arphaman, martong. Herald added a reviewer: shafik. jyknight requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This builtin

[PATCH] D119271: CGCall: also emit LLVM `allocalign` attribute when handling AllocAlign

jyknight added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:4605 + TryEmitAsCallSiteAttribute(const llvm::AttributeList &Attrs) { +llvm::AttributeList NewAttrs = Attrs; +if (AA) durin42 wrote: > jyknight wrote: > > We do need to fallback to

[PATCH] D120159: [Clang] Implement __builtin_source_location.

jyknight updated this revision to Diff 411692. jyknight added a comment. Minor tweaks to comments and docs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120159/new/ https://reviews.llvm.org/D120159 Files: clang/docs/LanguageExtensions.rst cla

[PATCH] D120159: [Clang] Implement __builtin_source_location.

jyknight updated this revision to Diff 411811. jyknight marked 15 inline comments as done. jyknight added a comment. Update per review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120159/new/ https://reviews.llvm.org/D120159 Files: cl

[PATCH] D120159: [Clang] Implement __builtin_source_location.

jyknight added a comment. In D120159#3341224 , @aaron.ballman wrote: > Btw, I think there may be some functionality missing for AST dumping, so I'd > like to see some additional tests for that. I'm not sure what test would actually show a problem (or l

[PATCH] D120159: [Clang] Implement __builtin_source_location.

jyknight marked 8 inline comments as done. jyknight added a comment. In D120159#3349069 , @aaron.ballman wrote: > Ah, okay, that's a good point. I was thinking this would show up in the AST > in places like: > > template > auto func() -> Ty; > >

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

jyknight added a comment. In D119051#3316026 , @dblaikie wrote: > Ah, looks like this is the existing > https://github.com/itanium-cxx-abi/cxx-abi/issues/66 If you're going to change the ABI, you might as well tackle the rest of the differences mention

[PATCH] D120159: [Clang] Implement __builtin_source_location.

jyknight updated this revision to Diff 412087. jyknight marked an inline comment as done. jyknight added a comment. Fix and test __impl lookup within the definition of std::source_location. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120159/new/

[PATCH] D115253: [C2x] Support the *_WIDTH macros in limits.h and stdint.h

jyknight added inline comments. Comment at: clang/lib/Frontend/InitPreprocessor.cpp:900-903 + Builder.defineMacro("__USHRT_WIDTH__", Twine(TI.getShortWidth())); + Builder.defineMacro("__UINT_WIDTH__", Twine(TI.getIntWidth())); + Builder.defineMacro("__ULONG_WIDTH__", Twine(TI.

[PATCH] D115253: [C2x] Support the *_WIDTH macros in limits.h and stdint.h

jyknight added inline comments. Comment at: clang/lib/Frontend/InitPreprocessor.cpp:900-903 + Builder.defineMacro("__USHRT_WIDTH__", Twine(TI.getShortWidth())); + Builder.defineMacro("__UINT_WIDTH__", Twine(TI.getIntWidth())); + Builder.defineMacro("__ULONG_WIDTH__", Twine(TI.

[PATCH] D115253: [C2x] Support the *_WIDTH macros in limits.h and stdint.h

jyknight accepted this revision. jyknight added a comment. Accepting assuming the last comment will be addressed before pushing. Thanks! Comment at: clang/lib/Frontend/InitPreprocessor.cpp:257 + if (IsSigned) +DefineTypeSizeAndWidth("__INT" + Twine(TypeWidth), Ty, TI, Buil

[PATCH] D87279: [clang] Fix handling of physical registers in inline assembly operands.

jyknight added a comment. This change seems like it's done in the wrong layer -- why do we add a special-case in Clang to emit "={r11},{r11}", instead of fixing LLVM to not break when given the previously-output "={r11},0"? Furthermore, since earlyclobber was exempted from this change, doesn't

[PATCH] D115471: [clang] number labels in asm goto strings after tied inputs

jyknight added a comment. It's rather sad that GCC made the quite-unintuitive decision to number the arguments in this way -- LONG AFTER clang had already implemented the other way... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115471/new/ http

[PATCH] D115409: [SelectionDAGBuilder] drop special handling for CallBr

jyknight added a comment. OK, I do think that special case definitely needs to be deleted. It's assuming that the block args are in a particular place in the argument list of the callbr -- the place Clang put them -- and then assigned special behavior based on that location. But I think it shou

[PATCH] D115410: [llvm][test] rewrite callbr to use i rather than X constraint NFC

jyknight added a comment. This looks good to me, but I'd like to wait for a conclusion on D115409 first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115410/new/ https://reviews.llvm.org/D115410 ___

[PATCH] D115311: [clang][CGStmt] emit i constraint rather than X for asm goto indirect dests

jyknight added a comment. This looks reasonable to me, but I'd like to wait for a conclusion on D115409 first. (Which probably will result in rewriting the commit message, as well). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D115410: [llvm][test] rewrite callbr to use i rather than X constraint NFC

jyknight added inline comments. Comment at: llvm/test/Verifier/callbr.ll:42 + callbr void asm sideeffect "${0:l} ${1:l} ${2:l}", "i,X,i"(i8* blockaddress(@test3, %4), i8* blockaddress(@test3, %2), i8* blockaddress(@test3, %3)) to label %1 [label %3, label %4] 1: --

[PATCH] D115253: [C2x] Support the *_WIDTH macros in limits.h and stdint.h

jyknight added inline comments. Comment at: clang/lib/Frontend/InitPreprocessor.cpp:900-903 + Builder.defineMacro("__USHRT_WIDTH__", Twine(TI.getShortWidth())); + Builder.defineMacro("__UINT_WIDTH__", Twine(TI.getIntWidth())); + Builder.defineMacro("__ULONG_WIDTH__", Twine(TI.

[PATCH] D115410: [llvm][test] rewrite callbr to use i rather than X constraint NFC

jyknight added inline comments. Comment at: llvm/test/tools/llvm-diff/callbr.ll:28-29 entry: - callbr void asm sideeffect "", "X,X,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %t_no), i8* blockaddress(@foo, %return)) + callbr void asm sideeffect "", "i,i,~{dirflag},~{f

[PATCH] D17183: Make TargetInfo store an actual DataLayout instead of a string.

jyknight added a comment. In general, I think it's extremely unfortunate that Clang and LLVM have different copies of the same information. It's a problem for way more than just this one situation. So, I really don't like choice 1 -- I think it's moving in the wrong direction. The recent threa

[PATCH] D99790: [CGCall] Annotate `this` argument with alignment

jyknight added a comment. In D99790#2677917 , @brooksmoses wrote: > As a heads up, I'm seeing segfaults on internal code as a result of this > change, as well as errors in Eigen's unalignedassert.cpp test (specifically, > this line asserts: > https://gi

[PATCH] D99790: [CGCall] Annotate `this` argument with alignment

jyknight added a comment. It seems like there's a bug with vtable thunks getting the wrong information. This appears to be a pre-existing bug, but this change has caused it to start actively breaking code. Test case: class Base1 { virtual void Foo1(); }; class Base2 { virt

[PATCH] D99790: [CGCall] Annotate `this` argument with alignment

jyknight added a comment. The OpenJDK bug was UB in the OpenJDK code: https://bugs.openjdk.java.net/browse/JDK-8229258 already fixed in JDK14. (But not backported to JDK 11 LTS, which is the version Brooks found an error in above.) They probably need to backport that patch. My only confusion

[PATCH] D93072: Fix PR35902: incorrect alignment used for ubsan check.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4ddf140c0040: Fix PR35902: incorrect alignment used for ubsan check. (authored by jyknight). Changed prior to commit: https://reviews.llvm.org/D93

[PATCH] D93922: Mangle `__alignof__` differently than `alignof`.

jyknight created this revision. jyknight added reviewers: rsmith, rjmccall. jyknight requested review of this revision. Herald added projects: clang, libc++abi, LLVM. Herald added subscribers: llvm-commits, libcxx-commits, cfe-commits. Herald added a reviewer: libc++abi. The two operations have ac

[PATCH] D86855: Convert __m64 intrinsics to unconditionally use SSE2 instead of MMX instructions.

jyknight marked 7 inline comments as done. jyknight added a comment. Herald added a subscriber: pengfei. I've finally got back to moving this patch forward -- PTAL, thanks! To start with, I wrote a simple test-suite to verify the functionality of these changes. I've included the tests I wrote un

[PATCH] D94213: Clang: Remove support for 3DNow!, both intrinsics and builtins.

jyknight created this revision. jyknight added reviewers: craig.topper, spatel, RKSimon. Herald added subscribers: dang, pengfei. jyknight requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. This set of instructions was only s

[PATCH] D94252: Delete (most) of the MMX builtin functions from Clang.

jyknight created this revision. jyknight added reviewers: craig.topper, spatel, RKSimon. jyknight requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. After switching the headers to implement the intrinsics using SSE2 (see http

[PATCH] D94268: Allow _mm_empty() (via llvm.x86.mmx.emms) to be a no-op without MMX.

jyknight created this revision. jyknight added reviewers: craig.topper, spatel, RKSimon. Herald added subscribers: pengfei, hiraditya. jyknight requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. In Clang, the other "MMX" intr

[PATCH] D94268: Allow _mm_empty() (via llvm.x86.mmx.emms) to be a no-op without MMX.

jyknight added a comment. In D94268#2485958 , @pengfei wrote: > Is inline assembly the only case `emms` instruction will be needed? But > inline assembly doesn't enable `mmx` attribute automatically, right? E.g. > https://godbolt.org/z/43ases Yes, inlin

[PATCH] D94268: Allow _mm_empty() (via llvm.x86.mmx.emms) to be a no-op without MMX.

jyknight abandoned this revision. jyknight added a comment. OK thanks -- abandoning this patch. I'll adjust the comment on _mm_empty to mention that it's no longer necessary except with asm in the intrinsics patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D94213: Clang: Remove support for 3DNow!, both intrinsics and builtins.

jyknight added a comment. Agreed w.r.t. timing -- I would like to get all of these changes landed soon after the LLVM 12 branch-cut, to allow plenty time to stew on the main branch before they make it into a release to try to identify any issues. I'd still appreciate review and approval with th

[PATCH] D93922: Mangle `__alignof__` differently than `alignof`.

jyknight added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:4009 +// mangling. Previously, it used a special-cased nonstandard extension. +if (Context.getASTContext().getLangOpts().getClangABICompat() >= +LangOptions::ClangABI::Ver11) { ---

[PATCH] D93922: Mangle `__alignof__` differently than `alignof`.

jyknight added a comment. Was the "group" libc++abi accept intended to be an accept for the whole revision? Or should still ping for review from rsmith or someone else? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93922/new/ https://reviews.llvm.

[PATCH] D106577: [clang] Define __STDC_ISO_10646__

jyknight added a comment. In D106577#2944086 , @aaron.ballman wrote: >> I don't think that scenario is valid. MBCS-to-unicode mappings are a part of >> the definition of the MBCS (sometimes officially, sometimes de-facto defined >> by major vendors), n

[PATCH] D108643: Introduce _BitInt, deprecate _ExtInt

jyknight added inline comments. Comment at: clang/include/clang/Basic/TargetInfo.h:581 /// limitation is put into place for ABI reasons. - virtual bool hasExtIntType() const { + /// FIXME: _BitInt is a required type in C23, so there's not much utility in + /// asking whethe

[PATCH] D108243: Revert "Avoid needlessly copying a block to the heap when a block literal"

jyknight added a comment. Ping. Any comment from Apple folks? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108243/new/ https://reviews.llvm.org/D108243 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D108243: Put code that avoids heapifying local blocks behind a flag

jyknight added a comment. In D108243#2995898 , @waltl wrote: > Added driver flags, and tests for them @ahatanak did you intend to ask Walt to add a driver flag for this? I think we should not have one, since this isn't something we should be telling us

[PATCH] D108243: Put code that avoids heapifying local blocks behind a flag

jyknight accepted this revision. jyknight added inline comments. Comment at: clang/include/clang/Driver/Options.td:2380 + NegFlag, + BothFlags<[CC1Option], " to avoid heapifying local blocks">>; Needs to be marked `[CC1Option, NoDriverOption]` Repository:

<    4   5   6   7   8   9