Page MenuHomePhabricator

jyknight (James Y Knight)
User

Projects

User does not belong to any projects.

User Details

User Since
Mar 27 2015, 11:23 AM (315 w, 2 d)

Recent Activity

Fri, Apr 9

jyknight added a comment to D99790: [CGCall] Annotate `this` argument with alignment.

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

Fri, Apr 9, 6:24 PM · Restricted Project, Restricted Project
jyknight added a comment to D99790: [CGCall] Annotate `this` argument with alignment.

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.

Fri, Apr 9, 4:20 PM · Restricted Project, Restricted Project

Thu, Apr 8

jyknight added a comment to D99790: [CGCall] Annotate `this` argument with alignment.

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://github.com/madlib/eigen/blob/master/test/unalignedassert.cpp#L151).

Thu, Apr 8, 2:48 PM · Restricted Project, Restricted Project

Tue, Apr 6

jyknight committed rG3b1b1d7530e6: Fix f6ee97d8271e1dfd9b6572222fefe8f40433952e: (authored by jyknight).
Fix f6ee97d8271e1dfd9b6572222fefe8f40433952e:
Tue, Apr 6, 11:12 AM

Mon, Apr 5

jyknight added inline comments to D98926: [sanitizer] Simplify GetTls with dl_iterate_phdr on Linux and use it on musl/FreeBSD.
Mon, Apr 5, 4:01 PM · Restricted Project

Mon, Mar 29

jyknight added a comment to D17183: Make TargetInfo store an actual DataLayout instead of a string..

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.

Mon, Mar 29, 12:59 PM

Mar 12 2021

jyknight added a comment to D97224: Use Address for CGBuilder's CreateAtomicRMW and CreateAtomicCmpXchg..

I'm less certain about what to do with the __atomic_* builtins

The __atomic builtins have already been supporting under-aligned pointers all along -- and that behavior is unchanged by this patch.

How so? Clang hasn't been passing down an alignment, which means that it's been building atomicrmw instructions with the natural alignment for the IR type, which means we've been assuming that all pointers have a least that alignment. The LLVM documentation even says that atomicrmw doesn't allow under-alignment.

Mar 12 2021, 9:30 AM · Restricted Project

Mar 11 2021

jyknight updated the diff for D97224: Use Address for CGBuilder's CreateAtomicRMW and CreateAtomicCmpXchg..

Use natural alignment for _Interlocked* intrinsics.

Mar 11 2021, 3:45 PM · Restricted Project
jyknight added a comment to D86855: Convert __m64 intrinsics to unconditionally use SSE2 instead of MMX instructions..

Or, if you have suggestions on how to make it easier to review, I'd be open to that.

Mar 11 2021, 8:25 AM · Restricted Project, Restricted Project

Mar 4 2021

jyknight updated the diff for D97984: ARM: Set MaxAtomicSizeInBitsSupported appropriately..

Version where tests actually pass: I forgot to merge a fixup commit before uploading.

Mar 4 2021, 3:21 PM · Restricted Project
jyknight requested review of D97987: AArch64: Set MaxAtomicSizeInBitsSupported appropriately..
Mar 4 2021, 3:20 PM · Restricted Project
jyknight requested review of D97984: ARM: Set MaxAtomicSizeInBitsSupported appropriately..
Mar 4 2021, 2:30 PM · Restricted Project
jyknight added a comment to D97224: Use Address for CGBuilder's CreateAtomicRMW and CreateAtomicCmpXchg..

The idea here is not to "ignore type alignment". EmitPointerWithAlignment will sometimes return an alignment for a pointer that's less than the alignment of the pointee type, e.g. because you're taking the address of a packed struct field. The critical question is whether the atomic builtins ought to make an effort to honor that reduced alignment, even if it leads to terrible code, or if we should treat the use of the builtin as a user promise that the pointer is actually more aligned than you might think from the information statically available.

Mar 4 2021, 12:57 PM · Restricted Project
jyknight added a comment to D97224: Use Address for CGBuilder's CreateAtomicRMW and CreateAtomicCmpXchg..

Do we really consider the existing atomic intrinsics to not impose added alignment restrictions? I'm somewhat concerned that we're going to produce IR here that's either really suboptimal or, worse, unimplementable, just because we over-interpreted some cue about alignment. I guess it would only be a significant problem on a target where types are frequently under-aligned for what atomics need, which is not typical, or when the user is doing atomics on a field of something like a packed struct.

Mar 4 2021, 9:56 AM · Restricted Project
jyknight updated the diff for D97224: Use Address for CGBuilder's CreateAtomicRMW and CreateAtomicCmpXchg..

Address review comments.

Mar 4 2021, 9:53 AM · Restricted Project

Feb 26 2021

jyknight committed rG6de6455752c1: Use getAlign() on atomicrmw/cmpxchg instructions, now that it's available. (authored by jyknight).
Use getAlign() on atomicrmw/cmpxchg instructions, now that it's available.
Feb 26 2021, 12:07 PM
jyknight added inline comments to D97223: Add Alignment argument to IRBuilder CreateAtomicRMW and CreateAtomicCmpXchg..
Feb 26 2021, 8:08 AM · Restricted Project, Restricted Project, Restricted Project
jyknight committed rG740e69b6fdc2: Fix assert to use getTypeStoreSize instead of getPrimitiveSizeInBits, (authored by jyknight).
Fix assert to use getTypeStoreSize instead of getPrimitiveSizeInBits,
Feb 26 2021, 8:08 AM

Feb 25 2021

jyknight accepted D97268: [X86] Use correct padding when in 16-bit mode.
Feb 25 2021, 7:35 PM · Restricted Project, Restricted Project
jyknight added a comment to D91256: [AtomicExpandPass] Allow for pointer types in insertRMWCmpXchgLoop().

Sorry for never looking at this before now. :(

Feb 25 2021, 3:39 PM · Restricted Project
jyknight committed rG24539f1ef247: Add Alignment argument to IRBuilder CreateAtomicRMW and CreateAtomicCmpXchg. (authored by jyknight).
Add Alignment argument to IRBuilder CreateAtomicRMW and CreateAtomicCmpXchg.
Feb 25 2021, 3:30 PM
jyknight closed D97223: Add Alignment argument to IRBuilder CreateAtomicRMW and CreateAtomicCmpXchg..
Feb 25 2021, 3:30 PM · Restricted Project, Restricted Project, Restricted Project
jyknight added a comment to D97438: [lld-macho] Change loadReexport to handle the case where a TAPI re-exports to reference documents nested within other TBD..

IIUC, the requirement is:

Feb 25 2021, 3:28 PM · Restricted Project, Restricted Project

Feb 24 2021

jyknight added a comment to D97268: [X86] Use correct padding when in 16-bit mode.

Please reduce the test case per MaskRay's comment, then lg.

Feb 24 2021, 2:50 PM · Restricted Project, Restricted Project
jyknight committed rGc2487bf7dfdd: Remove a workaround for MSVC 2013, now that MSVC 2017 is the minimum. (authored by jyknight).
Remove a workaround for MSVC 2013, now that MSVC 2017 is the minimum.
Feb 24 2021, 10:58 AM
jyknight added a comment to D97268: [X86] Use correct padding when in 16-bit mode.

That...shouldn't matter since a NOP is a NOP. Regardless, I removed that modification to simplify the patch.

Feb 24 2021, 6:28 AM · Restricted Project, Restricted Project

Feb 23 2021

jyknight accepted D97324: [NFC] Make TrailingObjects non-copyable/non-movable.

I can imagine there being some cases where these could theoretically be useful.

Feb 23 2021, 3:07 PM · Restricted Project
jyknight added a comment to D97268: [X86] Use correct padding when in 16-bit mode.

This change also modifies (breaks!) the 32-bit NOP set, definitely shouldn't do that. (But I think all that can be reverted, in any case).

Which part did it break?

Feb 23 2021, 3:04 PM · Restricted Project, Restricted Project
jyknight added a comment to D97223: Add Alignment argument to IRBuilder CreateAtomicRMW and CreateAtomicCmpXchg..

Looks like you didn't update the .bc reader/writer and the .ll printer/parser.

Feb 23 2021, 5:52 AM · Restricted Project, Restricted Project, Restricted Project
jyknight added a comment to D97268: [X86] Use correct padding when in 16-bit mode.

This change also modifies (breaks!) the 32-bit NOP set, definitely shouldn't do that. (But I think all that can be reverted, in any case).

Feb 23 2021, 5:43 AM · Restricted Project, Restricted Project

Feb 22 2021

jyknight committed rGe8617f2f1870: DebugInfo: Emit "LocalToUnit" flag on local member function decls. (authored by jyknight).
DebugInfo: Emit "LocalToUnit" flag on local member function decls.
Feb 22 2021, 3:47 PM
jyknight committed rGfe2dcd89acfd: DebugInfo: Emit "LocalToUnit" flag on local member function decls. (authored by jyknight).
DebugInfo: Emit "LocalToUnit" flag on local member function decls.
Feb 22 2021, 2:56 PM
jyknight closed D96044: DebugInfo: Emit "LocalToUnit" flag on local member function decls..
Feb 22 2021, 2:55 PM · Restricted Project
jyknight added a comment to D86855: Convert __m64 intrinsics to unconditionally use SSE2 instead of MMX instructions..

Ping.

Feb 22 2021, 1:35 PM · Restricted Project, Restricted Project
jyknight added a comment to D94252: Delete (most) of the MMX builtin functions from Clang..

Ping.

Feb 22 2021, 1:35 PM · Restricted Project, Restricted Project
jyknight added a comment to D94213: Clang: Remove support for 3DNow!, both intrinsics and builtins..

Ping.

Feb 22 2021, 1:35 PM · Restricted Project, Restricted Project
jyknight requested review of D97224: Use Address for CGBuilder's CreateAtomicRMW and CreateAtomicCmpXchg..
Feb 22 2021, 1:29 PM · Restricted Project
jyknight requested review of D97223: Add Alignment argument to IRBuilder CreateAtomicRMW and CreateAtomicCmpXchg..
Feb 22 2021, 1:29 PM · Restricted Project, Restricted Project, Restricted Project

Feb 12 2021

jyknight committed rG8bd8534aa3bd: LLVM-C: Allow LLVM{Get/Set}Alignment on an atomicrmw/cmpxchg instruction. (authored by jyknight).
LLVM-C: Allow LLVM{Get/Set}Alignment on an atomicrmw/cmpxchg instruction.
Feb 12 2021, 3:32 PM
jyknight added a comment to D92808: [ObjC][ARC] Use operand bundle 'clang.arc.attachedcall' instead of explicitly emitting retainRV or claimRV calls in the IR.

I ended up reverting the changes I made to llvm/lib/IR/AutoUpgrade.cpp as the file was including llvm/Analysis/ObjCARCUtil.h, which was violating layering.

Feb 12 2021, 11:55 AM · Restricted Project, Restricted Project
jyknight committed rG3c06676de14d: Fix layering after ed4718eccb12. (authored by jyknight).
Fix layering after ed4718eccb12.
Feb 12 2021, 11:55 AM

Feb 11 2021

jyknight committed rGdb00953ff32a: Fix bitcode decoder error in "Encode alignment attribute for `atomicrmw`" (authored by jyknight).
Fix bitcode decoder error in "Encode alignment attribute for `atomicrmw`"
Feb 11 2021, 7:30 PM
jyknight committed rG8043d5a9643b: NFC: update clang tests to check ordering and alignment for atomicrmw/cmpxchg. (authored by jyknight).
NFC: update clang tests to check ordering and alignment for atomicrmw/cmpxchg.
Feb 11 2021, 2:35 PM
jyknight committed rG17517f3178b5: Encode alignment attribute for `cmpxchg` (authored by gchatelet).
Encode alignment attribute for `cmpxchg`
Feb 11 2021, 12:18 PM
jyknight committed rGd06ab7981678: Encode alignment attribute for `atomicrmw` (authored by gchatelet).
Encode alignment attribute for `atomicrmw`
Feb 11 2021, 12:18 PM
jyknight closed D87443: Encode alignment attribute for `cmpxchg`.
Feb 11 2021, 12:18 PM · Restricted Project
jyknight closed D83465: Encode alignment attribute for `atomicrmw`.
Feb 11 2021, 12:18 PM · Restricted Project
jyknight accepted D87443: Encode alignment attribute for `cmpxchg`.

This seems to have fallen off everyone's radar -- sorry about that!

Feb 11 2021, 11:40 AM · Restricted Project
jyknight accepted D83465: Encode alignment attribute for `atomicrmw`.

This seems to have fallen off everyone's radar -- sorry about that!

Feb 11 2021, 11:40 AM · Restricted Project

Feb 10 2021

jyknight added inline comments to D95913: [lld-macho] Implement -bundle_loader.
Feb 10 2021, 11:22 AM · Restricted Project, Restricted Project

Feb 5 2021

jyknight added inline comments to D95913: [lld-macho] Implement -bundle_loader.
Feb 5 2021, 11:47 AM · Restricted Project, Restricted Project

Feb 4 2021

jyknight requested review of D96044: DebugInfo: Emit "LocalToUnit" flag on local member function decls..
Feb 4 2021, 8:16 AM · Restricted Project

Feb 3 2021

jyknight added a comment to D95913: [lld-macho] Implement -bundle_loader.

Please prefix the title of the commit with "[lld-macho]" so that it's easier to understand what bundle_loader is being implemented in.

Feb 3 2021, 3:43 PM · Restricted Project, Restricted Project
jyknight added a reviewer for D95913: [lld-macho] Implement -bundle_loader: int3.
Feb 3 2021, 3:13 PM · Restricted Project, Restricted Project

Feb 2 2021

jyknight added a comment to D71726: Let clang atomic builtins fetch add/sub support floating point types.

For amdgpu target, we do need diagnose unsupported atomics (not limited to fp atomics) since we do not support libcall due to ISA level linking not supported. This is something we cannot fix in a short time and we would rather diagnose it than confusing the users with missing symbols in lld.

Feb 2 2021, 11:58 AM · Restricted Project
jyknight added a comment to D71726: Let clang atomic builtins fetch add/sub support floating point types.

My concern is that this is treating a backend _bug_ as if it were just an optional feature. But it's not the case that it might be reasonable to either implement or not implement this in a backend -- it should be implemented, and those that don't are buggy.

Feb 2 2021, 9:38 AM · Restricted Project

Feb 1 2021

jyknight added a comment to D71726: Let clang atomic builtins fetch add/sub support floating point types.

I still have the same fundamental objection as before to the parts of this patch for prohibiting FP add/sub on some targets.

Feb 1 2021, 11:51 AM · Restricted Project

Jan 31 2021

jyknight committed rG20b1c1300c8f: Fix test in "CFG: Create scope for non-compound range-for body." (authored by jyknight).
Fix test in "CFG: Create scope for non-compound range-for body."
Jan 31 2021, 4:57 PM
jyknight committed rG8f670d5b6d8f: CFG: Create scope for non-compound range-for body. (authored by jyknight).
CFG: Create scope for non-compound range-for body.
Jan 31 2021, 3:43 PM

Jan 27 2021

jyknight committed rGa7246ba02a89: Itanium Mangling: In 'enable_if', omit X/E around <expr-primary>. (authored by jyknight).
Itanium Mangling: In 'enable_if', omit X/E around <expr-primary>.
Jan 27 2021, 1:53 PM
jyknight committed rG8ca33605ff0c: Itanium Mangling: Fix handling of <expr-primary> in <template-arg>. (authored by jyknight).
Itanium Mangling: Fix handling of <expr-primary> in <template-arg>.
Jan 27 2021, 1:53 PM
jyknight committed rG9c7aeaebb3ac: Itanium Mangling: Mangle `__alignof__` differently than `alignof`. (authored by jyknight).
Itanium Mangling: Mangle `__alignof__` differently than `alignof`.
Jan 27 2021, 1:53 PM
jyknight closed D95488: Itanium Mangling: In 'enable_if', omit X/E around <expr-primary>..
Jan 27 2021, 1:52 PM · Restricted Project
jyknight closed D95487: Itanium Mangling: Fix handling of <expr-primary> in <template-arg>..
Jan 27 2021, 1:52 PM · Restricted Project
jyknight closed D93922: Itanium Mangling: Mangle `__alignof__` differently than `alignof`..
Jan 27 2021, 1:52 PM · Restricted Project, Restricted Project, Restricted Project
jyknight added inline comments to D95487: Itanium Mangling: Fix handling of <expr-primary> in <template-arg>..
Jan 27 2021, 1:49 PM · Restricted Project
jyknight added inline comments to D95487: Itanium Mangling: Fix handling of <expr-primary> in <template-arg>..
Jan 27 2021, 12:15 PM · Restricted Project
jyknight updated the diff for D95488: Itanium Mangling: In 'enable_if', omit X/E around <expr-primary>..

Add neglected clang-abi-compat.cpp change.

Jan 27 2021, 7:42 AM · Restricted Project
jyknight added inline comments to D95487: Itanium Mangling: Fix handling of <expr-primary> in <template-arg>..
Jan 27 2021, 6:43 AM · Restricted Project

Jan 26 2021

jyknight added a comment to D93922: Itanium Mangling: Mangle `__alignof__` differently than `alignof`..

OK, I've posted two follow-up changes now.

Jan 26 2021, 3:51 PM · Restricted Project, Restricted Project, Restricted Project
jyknight requested review of D95488: Itanium Mangling: In 'enable_if', omit X/E around <expr-primary>..
Jan 26 2021, 3:49 PM · Restricted Project
jyknight added reviewers for D95487: Itanium Mangling: Fix handling of <expr-primary> in <template-arg>.: rsmith, rjmccall.
Jan 26 2021, 3:48 PM · Restricted Project
jyknight requested review of D95487: Itanium Mangling: Fix handling of <expr-primary> in <template-arg>..
Jan 26 2021, 3:47 PM · Restricted Project
jyknight updated the diff for D93922: Itanium Mangling: Mangle `__alignof__` differently than `alignof`..

Minor updates.

Jan 26 2021, 3:47 PM · Restricted Project, Restricted Project, Restricted Project

Jan 21 2021

jyknight added inline comments to D93922: Itanium Mangling: Mangle `__alignof__` differently than `alignof`..
Jan 21 2021, 12:00 PM · Restricted Project, Restricted Project, Restricted Project
jyknight updated the diff for D93922: Itanium Mangling: Mangle `__alignof__` differently than `alignof`..

Address comments.

Jan 21 2021, 12:00 PM · Restricted Project, Restricted Project, Restricted Project

Jan 20 2021

jyknight added a comment to D93922: Itanium Mangling: Mangle `__alignof__` differently than `alignof`..

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?

Jan 20 2021, 8:59 AM · Restricted Project, Restricted Project, Restricted Project

Jan 14 2021

jyknight added a comment to D94542: [X86] Default to -x86-pad-for-align=false to drop assembler difference with or w/o -g.

This doesn't get at the root cause though. Are those labels doing anything in the debug build? Why are they emitted? Can they be reasonably removed?

Jan 14 2021, 3:05 PM · Restricted Project
jyknight added a comment to D94470: [LSR] Don't break a critical edge if parent ends with "callbr".

This patch also fixes the test case in D88438 (and D88438 also fixes this test case).

Jan 14 2021, 1:25 PM · Restricted Project
jyknight accepted D88438: BreakCriticalEdges: do not split the critical edge from a CallBr indirect successor.

Looks totally reasonable to me.

Jan 14 2021, 1:13 PM · Restricted Project
jyknight added inline comments to D93922: Itanium Mangling: Mangle `__alignof__` differently than `alignof`..
Jan 14 2021, 9:17 AM · Restricted Project, Restricted Project, Restricted Project

Jan 13 2021

jyknight added a comment to D75203: [X86] Relax existing instructions to reduce the number of nops needed for alignment purposes.

Right, there's no fundamental reason why moving a label has to be forbidden -- but it'd be extremely complex if we allowed moving a label which could cause the re-layout of a fragment we thought we'd already finalized the offsets for. This would happen if the label offset required relaxation of some instruction/data referencing it. That, then, might require undoing the padding from an instruction we've already padded out, due to less alignment-padding being required overall.

Jan 13 2021, 8:57 AM · Restricted Project

Jan 9 2021

jyknight added a comment to D94213: Clang: Remove support for 3DNow!, both intrinsics and builtins..

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.

Jan 9 2021, 11:53 AM · Restricted Project, Restricted Project

Jan 8 2021

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

OK thanks -- abandoning this patch.

Jan 8 2021, 7:38 PM · Restricted Project, Restricted Project
jyknight added a comment to D94268: Allow _mm_empty() (via llvm.x86.mmx.emms) to be a no-op without MMX..

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

Jan 8 2021, 1:34 PM · Restricted Project, Restricted Project

Jan 7 2021

jyknight requested review of D94268: Allow _mm_empty() (via llvm.x86.mmx.emms) to be a no-op without MMX..
Jan 7 2021, 2:27 PM · Restricted Project, Restricted Project
jyknight requested review of D94252: Delete (most) of the MMX builtin functions from Clang..
Jan 7 2021, 11:41 AM · Restricted Project, Restricted Project

Jan 6 2021

jyknight requested review of D94213: Clang: Remove support for 3DNow!, both intrinsics and builtins..
Jan 6 2021, 8:37 PM · Restricted Project, Restricted Project
jyknight updated the diff for D86855: Convert __m64 intrinsics to unconditionally use SSE2 instead of MMX instructions..

Fix and test.

Jan 6 2021, 8:30 PM · Restricted Project, Restricted Project
jyknight added a comment to D86855: Convert __m64 intrinsics to unconditionally use SSE2 instead of MMX instructions..

I've finally got back to moving this patch forward -- PTAL, thanks!

Jan 6 2021, 8:30 PM · Restricted Project, Restricted Project

Dec 29 2020

jyknight requested review of D93922: Itanium Mangling: Mangle `__alignof__` differently than `alignof`..
Dec 29 2020, 2:29 PM · Restricted Project, Restricted Project, Restricted Project
jyknight committed rG145cbef5879c: Copy demangle changes from libcxxabi to llvm with cp_to_llvm.sh. (authored by jyknight).
Copy demangle changes from libcxxabi to llvm with cp_to_llvm.sh.
Dec 29 2020, 1:18 PM

Dec 28 2020

jyknight committed rG4ddf140c0040: Fix PR35902: incorrect alignment used for ubsan check. (authored by jyknight).
Fix PR35902: incorrect alignment used for ubsan check.
Dec 28 2020, 3:33 PM
jyknight closed D93072: Fix PR35902: incorrect alignment used for ubsan check..
Dec 28 2020, 3:33 PM · Restricted Project

Dec 10 2020

jyknight requested review of D93072: Fix PR35902: incorrect alignment used for ubsan check..
Dec 10 2020, 3:10 PM · Restricted Project
jyknight added a comment to D92662: [Clang][Coroutine] Drop const attribute on pthread_self when coroutine is enabled.

I don't think we should change the meaning of __attribute__((const)) to exclude depending on thread-id.

Dec 10 2020, 12:40 PM · Restricted Project

Dec 9 2020

jyknight added inline comments to D91311: Add new 'preferred_name' attribute..
Dec 9 2020, 7:13 AM · Restricted Project, Restricted Project

Dec 1 2020

jyknight added a comment to D92375: [MergeICmps] Fix missing split..

Oh -- if it's not too late, can you fix the commit message before pushing -- it could probably do with a little more explanation.

Dec 1 2020, 7:29 AM · Restricted Project
jyknight accepted D92375: [MergeICmps] Fix missing split..
Dec 1 2020, 7:01 AM · Restricted Project
jyknight added a reviewer for D92375: [MergeICmps] Fix missing split.: jyknight.
Dec 1 2020, 6:20 AM · Restricted Project