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 (303 w, 4 d)

Recent Activity

Thu, Jan 14

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?

Thu, Jan 14, 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).

Thu, Jan 14, 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.

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

Wed, Jan 13

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.

Wed, Jan 13, 8:57 AM · Restricted Project

Sat, Jan 9

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.

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

Fri, Jan 8

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

OK thanks -- abandoning this patch.

Fri, Jan 8, 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

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

Thu, Jan 7

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

Wed, Jan 6

jyknight requested review of D94213: Clang: Remove support for 3DNow!, both intrinsics and builtins..
Wed, Jan 6, 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.

Wed, Jan 6, 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!

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

Tue, Dec 29

jyknight requested review of D93922: Mangle `__alignof__` differently than `alignof`..
Tue, Dec 29, 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.
Tue, Dec 29, 1:18 PM

Mon, Dec 28

jyknight committed rG4ddf140c0040: Fix PR35902: incorrect alignment used for ubsan check. (authored by jyknight).
Fix PR35902: incorrect alignment used for ubsan check.
Mon, Dec 28, 3:33 PM
jyknight closed D93072: Fix PR35902: incorrect alignment used for ubsan check..
Mon, Dec 28, 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

Nov 19 2020

jyknight accepted D91157: [AArch64] Out-of-line atomics (-moutline-atomics) implementation..

LG after fixing the minor nits.

Nov 19 2020, 7:44 AM · Restricted Project, Restricted Project

Nov 18 2020

jyknight accepted D88625: MCExpr::evaluateAsRelocatableImpl : allow evaluation of non-VK_None MCSymbolRefExpr when MCAsmLayout is available.

AFAICT, this looks good now. I guess we'll see if anything breaks after it's submitted. =)

Nov 18 2020, 12:26 PM · Restricted Project

Nov 13 2020

jyknight added inline comments to D88625: MCExpr::evaluateAsRelocatableImpl : allow evaluation of non-VK_None MCSymbolRefExpr when MCAsmLayout is available.
Nov 13 2020, 7:16 AM · Restricted Project

Nov 12 2020

jyknight added inline comments to D88625: MCExpr::evaluateAsRelocatableImpl : allow evaluation of non-VK_None MCSymbolRefExpr when MCAsmLayout is available.
Nov 12 2020, 2:54 PM · Restricted Project

Nov 10 2020

jyknight added inline comments to D91156: [AArch64] Compiler-rt interface for out-of-line atomics..
Nov 10 2020, 2:31 PM · Restricted Project
jyknight added inline comments to D91157: [AArch64] Out-of-line atomics (-moutline-atomics) implementation..
Nov 10 2020, 2:27 PM · Restricted Project, Restricted Project
jyknight added a comment to D63744: In the libc++ unstable ABI, use [[no_unique_address]] instead of __compressed_pair when available..

Hm, that might indeed be feasible. We'll need to potentially insert padding both before the first type and after the second type, but we can static_assert the correctness against the old layout, so that's not _too_ scary.

Nov 10 2020, 7:15 AM · Restricted Project

Nov 2 2020

jyknight added a comment to D63744: In the libc++ unstable ABI, use [[no_unique_address]] instead of __compressed_pair when available..

If we assume that the compiler supports [[no_unique_address]], is this an ABI break? Does no_unique_address result in a different layout than the __compressed_pair?

Nov 2 2020, 2:59 PM · Restricted Project
jyknight added a comment to D88625: MCExpr::evaluateAsRelocatableImpl : allow evaluation of non-VK_None MCSymbolRefExpr when MCAsmLayout is available.

I have tried adding the expression folding stuff to`MCExpr::evaluateAsRelocatableImpl` but I abandoned that idea because evaluateAsRelocatableImpl does not like a modifier different from VK_None (@PLT). (What does a@plt + 1 do? Relaxing the condition (== VK_None) may work... but the semantics are not very clear.

a@plt + 1 is perfectly fine -- that turns into a R_X86_64_PLT32 relocation with symbol "a" and addend 1.

The interesting question is what this should do:

b=a+1 # no @plt!
... b@plt

It's certainly impossible to create a PLT referring to a function at address "a+1", in current object formats. So, probably this should be invalid. However, GAS seems to consider the modifier as more of an overall-modifier, resulting in the same thing as a@plt + 1. I agree that's probably not something we ought to support, since it seems wrong.

But, it does still seem reasonable to put the !VK_None support into evaluateAsRelocatableImpl -- as long as the resolved MCValue has only a single symbol, no offset and no kind.

The current way the patch implements the "if a fixup symbol is equated to an expression with an undefined symbol" logic is consistent with the step GNU as implements the check.
GNU as implements the logic when it is about to emit a relocation, not when folding expressions.

Nov 2 2020, 12:23 PM · Restricted Project

Oct 23 2020

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

clang does not always emit atomic instructions for atomic builtins. Clang may emit lib calls for atomic builtins. Basically clang checks target info about max atomic inline width and if the desired atomic operation exceeds the supported atomic inline width, clang will emit lib calls for atomic builtins. The rationale is that the lib calls may be faster than the IR generated by the LLVM pass. This behavior has long existed and it also applies to fp atomics. I don't think emitting lib calls for atomic builtins is a bug. However, this does introduce the issue about whether the library functions for atomics are available for a specific target. As I said, only the target owners have the answer and therefore I introduced the target hook.

Oct 23 2020, 11:38 AM
jyknight accepted D89891: [llvm-ar][Object] Fix detection of need for 64-bit archive symbol tables.

LGTM.

Oct 23 2020, 10:55 AM · Restricted Project

Oct 21 2020

jyknight added a comment to D89615: Disable emulated-tls and use LLD for compiler-rt+tests on Android.

Why we can't just switch to elf TLS?

Oct 21 2020, 11:43 AM · Restricted Project

Oct 19 2020

jyknight added a comment to D88659: [NFC] Fix the definition of SuitableAlign.

Hi @jyknight , are you okay with us changing the "definition" of SuitableAlign without sending a note to the mailing list?

Oct 19 2020, 11:08 AM · Restricted Project

Oct 13 2020

jyknight added a comment to D88625: MCExpr::evaluateAsRelocatableImpl : allow evaluation of non-VK_None MCSymbolRefExpr when MCAsmLayout is available.

I have tried adding the expression folding stuff to`MCExpr::evaluateAsRelocatableImpl` but I abandoned that idea because evaluateAsRelocatableImpl does not like a modifier different from VK_None (@PLT). (What does a@plt + 1 do? Relaxing the condition (== VK_None) may work... but the semantics are not very clear.

Oct 13 2020, 11:57 AM · Restricted Project

Oct 10 2020

jyknight added a comment to D88659: [NFC] Fix the definition of SuitableAlign.

It seems like on AIX, __BIGGEST_ALIGNMENT__ should just be set to 16, then. I'm not sure why you want it to be 8?

/// Return the alignment that is suitable for storing any
/// object with a fundamental alignment requirement.

Vector types have extended (not fundamental) alignment on AIX, because malloc is not guaranteed to return addresses that are 16-byte aligned.

Oct 10 2020, 2:03 PM · Restricted Project

Oct 7 2020

jyknight added a comment to D88659: [NFC] Fix the definition of SuitableAlign.

As you mentioned, without this patch, SuitableAlign is used for the predefined __BIGGEST_ALIGNMENT__ and alignment for alloca. But on AIX, the BIGGEST_ALIGNMENT should be 8bytes, alloca and __attribute__((aligned)) should align to 16bytes considering vector types which have to be aligned to 16bytes, that's why we want to split SuitableAlign following current Clang state. We'd like to split something out to consider vector type alignment.

Oct 7 2020, 3:56 PM · Restricted Project

Oct 6 2020

jyknight added a comment to D88823: [CodeGen][TailDuplicator] Don't duplicate blocks with INLINEASM_BR.

Okay. Abandoning this change.

Oct 6 2020, 8:41 AM · Restricted Project

Oct 5 2020

jyknight added a comment to D88823: [CodeGen][TailDuplicator] Don't duplicate blocks with INLINEASM_BR.

I disagree. You'll be inserting an instruction that won't be executed on the default path before the asm goto block. While this may be okay in a majority of cases, I don't suspect that it'll be good to rely on it being always okay.

Oct 5 2020, 11:04 PM · Restricted Project
jyknight added a comment to D88823: [CodeGen][TailDuplicator] Don't duplicate blocks with INLINEASM_BR.

When we go to resolve this PHI node, there's no place to put the put the values that works in all situations; we can't split the critical edges and it's not valid to place the resolved instructions at the ends of the predecessor blocks.

Oct 5 2020, 7:20 PM · Restricted Project
jyknight added a comment to D88823: [CodeGen][TailDuplicator] Don't duplicate blocks with INLINEASM_BR.

Hm, it doesn't seem like it should be a problem to duplicate an inlineasm_br instruction. I'm interested to see what's going wrong here.

Oct 5 2020, 9:49 AM · Restricted Project

Oct 1 2020

jyknight added a comment to D88659: [NFC] Fix the definition of SuitableAlign.

Hm, to start with, the current state of this confuses me.

Oct 1 2020, 9:20 AM · Restricted Project

Sep 29 2020

jyknight accepted D86790: [FE] Use preferred alignment instead of ABI alignment for complete object when applicable .
Sep 29 2020, 4:54 AM · Restricted Project

Sep 24 2020

jyknight accepted D88195: Remove stale assert..

Clarify commit message.

Phabricator unfortunately won't amend the review description when the commit message is updated; you'll need to manually edit the description via phab's UI for other reviewers to observe the update. :(

Sep 24 2020, 1:53 PM · Restricted Project
jyknight added inline comments to D88195: Remove stale assert..
Sep 24 2020, 7:31 AM · Restricted Project

Sep 23 2020

jyknight added inline comments to D88195: Remove stale assert..
Sep 23 2020, 8:08 PM · Restricted Project
jyknight added inline comments to D86790: [FE] Use preferred alignment instead of ABI alignment for complete object when applicable .
Sep 23 2020, 7:57 PM · Restricted Project
jyknight added a comment to D86790: [FE] Use preferred alignment instead of ABI alignment for complete object when applicable .

I'm happy with this now, but please update the commit message to match the updated change.

Sep 23 2020, 4:35 PM · Restricted Project
jyknight added inline comments to D86790: [FE] Use preferred alignment instead of ABI alignment for complete object when applicable .
Sep 23 2020, 4:17 PM · Restricted Project

Sep 18 2020

jyknight committed rGf7a53d82c090: PR47468: Fix findPHICopyInsertPoint, so that copies aren't incorrectly inserted… (authored by jyknight).
PR47468: Fix findPHICopyInsertPoint, so that copies aren't incorrectly inserted…
Sep 18 2020, 11:14 AM
jyknight closed D87865: PR47468: Fix findPHICopyInsertPoint, so that copies aren't incorrectly inserted after an INLINEASM_BR..
Sep 18 2020, 11:14 AM · Restricted Project
jyknight added inline comments to D86790: [FE] Use preferred alignment instead of ABI alignment for complete object when applicable .
Sep 18 2020, 9:55 AM · Restricted Project
jyknight updated the diff for D87865: PR47468: Fix findPHICopyInsertPoint, so that copies aren't incorrectly inserted after an INLINEASM_BR..

Comment nits.

Sep 18 2020, 9:42 AM · Restricted Project
jyknight added inline comments to D87865: PR47468: Fix findPHICopyInsertPoint, so that copies aren't incorrectly inserted after an INLINEASM_BR..
Sep 18 2020, 9:33 AM · Restricted Project

Sep 17 2020

jyknight published D87865: PR47468: Fix findPHICopyInsertPoint, so that copies aren't incorrectly inserted after an INLINEASM_BR. for review.
Sep 17 2020, 3:44 PM · Restricted Project

Sep 11 2020

jyknight added a comment to D86881: Make -fvisibility-inlines-hidden apply to static local variables in inline functions on Darwin.

But this (re-)breaks the functionality of -fvisibility-inlines-hidden on Darwin. That seems bad? I'd've liked to see more of an explanation as to why this was considered a necessary breakage.

Sep 11 2020, 11:31 AM · Restricted Project

Sep 3 2020

jyknight added a comment to D86790: [FE] Use preferred alignment instead of ABI alignment for complete object when applicable .

Do you have open questions on whether some callsites passing "false" here, should be switched to true? Given what's here, I would say that it definitely does not makes sense to add this parameter everywhere.

Sep 3 2020, 1:50 PM · Restricted Project

Aug 31 2020

jyknight added inline comments to D86815: [LangRef] Adjust guarantee for llvm.memcpy to also allow equal arguments..
Aug 31 2020, 5:49 PM · Restricted Project

Aug 30 2020

jyknight requested review of D86855: Convert __m64 intrinsics to unconditionally use SSE2 instead of MMX instructions..
Aug 30 2020, 3:29 PM · Restricted Project, Restricted Project

Aug 29 2020

jyknight added inline comments to D86260: [CodeGen] Postprocess PHI nodes for callbr.
Aug 29 2020, 3:35 PM · Restricted Project

Aug 28 2020

jyknight added a comment to D85225: [Target][AArch64] Allow for char as int8_t in AArch64AsmParser.cpp.

ro: In this case, I would say to defer to Paul, although I wish that he would change his mind.

Aug 28 2020, 3:35 PM · Restricted Project

Aug 25 2020

jyknight added a comment to D82777: Clang Driver: Use Apple ld64's new @response-file support..

BTW, to the apple folks here, it'd sure be awesome if this could be backported into XCode 12's clang. :) (c.f. FB7037642).

Aug 25 2020, 4:52 PM · Restricted Project

Aug 24 2020

jyknight added a comment to D86142: [LLD] Search archives for non-tentative defintions..

Given that BFD supports this behaviour but Gold and LLD do not I suspect that the risk of breaking existing programs is small, especially as LLD tends to be used with more recent programs where -fno-common is the default. However I think we should be cautious and perhaps enable under a command line option, especially as it looks like this is a convention rather than part of a specification. If anyone has better references, then I'm happy to retract.

I agree the new behavior needs an option to enable it and disable it. Initially I considered having the Fortran driver pass the option to LLD, but after discussing it offline with @rzurob I think it might be best to enable the behavior by default. The consumer of these libraries aren't necessarily Fortran programs, and as you mentioned its likely to be lower risk because we are adopting a behavior already implemented in ld.bfd.

There are 4 combinations: (ArchiveFile,LazyObjFile) x (ELF, BitcodeFile). LazyObjFile is for --start-lib/--end-lib. Do you think whether it is worth making LazyObjFile or BitcodeFile consistent with ArchiveFile x ELF behavior? If we add an option (say, --fortran-linking, the Dec 1999 binutils thread suggested but did not adopt), we will have better justification that we don't make LazyObjFile or BitcodeFile consistent.

Aug 24 2020, 5:20 PM · Restricted Project, Restricted Project
jyknight added inline comments to D85225: [Target][AArch64] Allow for char as int8_t in AArch64AsmParser.cpp.
Aug 24 2020, 1:51 PM · Restricted Project

Aug 20 2020

jyknight added a comment to D86260: [CodeGen] Postprocess PHI nodes for callbr.

The commit message is confusing for this -- this isn't a correctness fix, but a minor optimization, right?

I'm not convinced it's just an optimization, but I could be swayed. It seems to me that the value coming into the PHI shouldn't rely upon the semantics of the asm block. So for instance would this be incorrect?

Aug 20 2020, 3:57 PM · Restricted Project
jyknight added a comment to D86260: [CodeGen] Postprocess PHI nodes for callbr.

The commit message is confusing for this -- this isn't a correctness fix, but a minor optimization, right?

Aug 20 2020, 3:22 PM · Restricted Project
jyknight added a comment to D85044: Add __atomic_is_lock_free to compiler-rt.

There is going to be a bunch more complexity required here, I'm afraid.

Aug 20 2020, 12:47 PM · Restricted Project, Restricted Project

Aug 18 2020

jyknight added inline comments to D85225: [Target][AArch64] Allow for char as int8_t in AArch64AsmParser.cpp.
Aug 18 2020, 2:49 PM · Restricted Project
jyknight added inline comments to D85225: [Target][AArch64] Allow for char as int8_t in AArch64AsmParser.cpp.
Aug 18 2020, 8:52 AM · Restricted Project

Aug 17 2020

jyknight added a comment to D85630: [cmake] Don't build with -O3 -fPIC on Solaris/sparcv9.

Presumably this is a bug in something -- either the solaris linker, in g++, or in the llvm code being mis-compiled? It seems unfortunate to put in place a hacky workaround like this, without a bug reference to the responsible component.

Aug 17 2020, 2:28 PM · Restricted Project

Aug 10 2020

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

Oh, one more note, C11 has -- and clang already supports -- _Atomic long double x; x += 4; via lowering to a cmpxchg loop. Now that we have an LLVM IR representation for atomicrmw fadd/fsub, clang should be lowering the _Atomic += to that, too. (Doesn't need to be in this patch, but it should be done.)

Aug 10 2020, 9:13 AM
jyknight added a comment to D71726: Let clang atomic builtins fetch add/sub support floating point types.
In D71726#2182667, @tra wrote:

If a target would like to treat single and double fp atomics as unsupported, it can override the default behavior in its own TargetInfo.

Aug 10 2020, 9:11 AM

Jul 29 2020

jyknight added inline comments to D83465: Encode alignment attribute for `atomicrmw`.
Jul 29 2020, 3:38 PM · Restricted Project

Jul 27 2020

jyknight added a comment to D83533: [Alignment][NFC] Update Bitcodewriter to use Align.

I think this new header ought to go in include/llvm/Bitcode/, not Bitstream?

Jul 27 2020, 1:01 PM · Restricted Project

Jul 21 2020

jyknight added a comment to D84200: Disable trivial weak_ptr test on ARM because it is not expected to work..

Right, the trivial_abi attribute doesn't guarantee that you pass an object in registers, it just gives permission for the implementation to treat the struct as-if it was a trivial struct. On ARM32, since the ABI specifies that trivial structs > 4 bytes are returned indirectly by passing a hidden pointer to a pre-allocated return struct, that's what you get.

Jul 21 2020, 3:52 PM · Restricted Project
jyknight added a comment to D71726: Let clang atomic builtins fetch add/sub support floating point types.

Why not have clang always emit atomicrmw for floats, and let AtomicExpandPass handle legalizing that into integer atomics if necessary, rather than adding a target hook in clang?

Not all targets can legalize fp atomics by AtomicExpandPass. Some targets need library support.

Jul 21 2020, 3:41 PM
jyknight added a comment to D71726: Let clang atomic builtins fetch add/sub support floating point types.

Why not have clang always emit atomicrmw for floats, and let AtomicExpandPass handle legalizing that into integer atomics if necessary, rather than adding a target hook in clang?

Jul 21 2020, 3:05 PM

Jul 20 2020

jyknight accepted D84183: [libcxx] Skip tests on GCC.

This kind of change is usually ok to just push, with a note on the original review that you had to push a followup fix.

Jul 20 2020, 9:55 AM · Restricted Project

Jul 17 2020

jyknight committed rG8a438096ffa4: Remove TwoAddressInstructionPass::sink3AddrInstruction. (authored by jyknight).
Remove TwoAddressInstructionPass::sink3AddrInstruction.
Jul 17 2020, 1:31 PM

Jul 16 2020

jyknight committed rG60433c63acb7: Remove TwoAddressInstructionPass::sink3AddrInstruction. (authored by jyknight).
Remove TwoAddressInstructionPass::sink3AddrInstruction.
Jul 16 2020, 7:03 AM
jyknight closed D83708: Remove TwoAddressInstructinoPass::sink3AddrInstruction..
Jul 16 2020, 7:03 AM · Restricted Project

Jul 14 2020

jyknight added inline comments to D83708: Remove TwoAddressInstructinoPass::sink3AddrInstruction..
Jul 14 2020, 11:50 AM · Restricted Project

Jul 13 2020

jyknight added inline comments to D83708: Remove TwoAddressInstructinoPass::sink3AddrInstruction..
Jul 13 2020, 1:53 PM · Restricted Project
jyknight updated the diff for D83708: Remove TwoAddressInstructinoPass::sink3AddrInstruction..

Improve commit message wording
Simplify added test a bit

Jul 13 2020, 1:53 PM · Restricted Project
jyknight added inline comments to D83708: Remove TwoAddressInstructinoPass::sink3AddrInstruction..
Jul 13 2020, 1:26 PM · Restricted Project
jyknight added a comment to D83704: [llvm-readobj] Print error when executed with no input files.

FWIW, it rather bugs me when tools print their entire (long) help upon command-line syntax error. Simply printing an error is generally better. (E.g. "error: no input files specified.")

Jul 13 2020, 12:23 PM · Restricted Project
jyknight updated the summary of D83708: Remove TwoAddressInstructinoPass::sink3AddrInstruction..
Jul 13 2020, 12:01 PM · Restricted Project
jyknight added a comment to D83523: MachineSink: permit sinking into INLINEASM_BR indirect targets.

INLINEASM_BR already returns true for hasUnmodeledSideEffects(), so the diff above doesn't change anything.

Jul 13 2020, 11:53 AM · Restricted Project
Herald added a project to D83708: Remove TwoAddressInstructinoPass::sink3AddrInstruction.: Restricted Project.
Jul 13 2020, 11:51 AM · Restricted Project

Jul 10 2020

jyknight added a comment to D83523: MachineSink: permit sinking into INLINEASM_BR indirect targets.

It looks like the issue shown in this test-case appears in Two-Address instruction pass, not Machine Sink.

Jul 10 2020, 3:15 PM · Restricted Project

Jul 6 2020

jyknight added a comment to D82490: [libcxx] Put clang::trivial_abi on std::unique_ptr.

Of course, running into the problem also requires an asserts-enabled build of clang, because this change has triggered a Clang bug, which will need to be fixed before this can be recommitted.

Jul 6 2020, 1:12 PM · Restricted Project
jyknight added a comment to D83015: [Driver] Add --ld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path.

BTW, I just noticed recently that we have a -mlinker-version= flag, too, which is only used on darwin at the moment. That's another instance of "we need to condition behavior based on what linker we're invoking", but in this case, between multiple versions of apple's linker, rather than which brand of linker. That doesn't impact this directly, but just thought I'd mention it as it's in the same area of concern.

Jul 6 2020, 1:04 PM · Restricted Project
jyknight accepted D83136: [NFC] Adding the align attribute on Atomic{CmpXchg|RMW}Inst.

It's odd to have CreateAtomicCmpXchg and CreateAtomicRMW not have Align as an argument when the constructors do, but as long as the migration is finished in an upcoming commit, this seems fine as a step on the path.

Jul 6 2020, 12:55 PM · Restricted Project

Jul 1 2020

jyknight added a comment to D81369: [Alignment][NFC] Migrate AtomicExpandPass to Align.

Thanks for the cleanup!

Jul 1 2020, 3:41 PM · Restricted Project
jyknight added a comment to D82923: introducing llvm-libtool-darwin.

I suggest removing the extraneous level of directory "Darwin" -- just put the files in "llvm/tools/llvm-libtool-darwin", name the source "llvm-libtool-darwin.cpp", and say you're adding "llvm-libtool-darwin" in the commit message.

Jul 1 2020, 12:27 PM · Restricted Project
jyknight committed rG4b0aa5724fea: Change the INLINEASM_BR MachineInstr to be a non-terminating instruction. (authored by jyknight).
Change the INLINEASM_BR MachineInstr to be a non-terminating instruction.
Jul 1 2020, 10:16 AM
jyknight closed D79794: Change the INLINEASM_BR MachineInstr to be a non-terminating instruction..
Jul 1 2020, 10:16 AM · Restricted Project

Jun 29 2020

jyknight added inline comments to D79794: Change the INLINEASM_BR MachineInstr to be a non-terminating instruction..
Jun 29 2020, 4:22 PM · Restricted Project