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

2021-03-29 Thread James Y Knight via Phabricator via cfe-commits
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

2021-04-08 Thread James Y Knight via Phabricator via cfe-commits
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

2021-04-09 Thread James Y Knight via Phabricator via cfe-commits
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

2021-04-09 Thread James Y Knight via Phabricator via cfe-commits
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.

2020-12-28 Thread James Y Knight via Phabricator via cfe-commits
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`.

2020-12-29 Thread James Y Knight via Phabricator via cfe-commits
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.

2021-01-06 Thread James Y Knight via Phabricator via cfe-commits
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.

2021-01-06 Thread James Y Knight via Phabricator via cfe-commits
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.

2021-01-07 Thread James Y Knight via Phabricator via cfe-commits
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.

2021-01-07 Thread James Y Knight via Phabricator via cfe-commits
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.

2021-01-08 Thread James Y Knight via Phabricator via cfe-commits
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.

2021-01-08 Thread James Y Knight via Phabricator via cfe-commits
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.

2021-01-09 Thread James Y Knight via Phabricator via cfe-commits
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`.

2021-01-14 Thread James Y Knight via Phabricator via cfe-commits
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`.

2021-01-20 Thread James Y Knight via Phabricator via cfe-commits
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__

2021-08-20 Thread James Y Knight via Phabricator via cfe-commits
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

2021-08-24 Thread James Y Knight via Phabricator via cfe-commits
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"

2021-08-26 Thread James Y Knight via Phabricator via cfe-commits
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

2021-09-13 Thread James Y Knight via Phabricator via cfe-commits
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

2021-09-13 Thread James Y Knight via Phabricator via cfe-commits
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:

<    1   2   3   4   5