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

Recent Activity

Yesterday

jyknight added a comment to D54653: Remove branch from CreateAlignmentAssumption.

Thus, I think that the simple expression will still work even with this use case.

Indeed so!

I think the rule can be more clearly stated as: When the pointer is non-null, then the alignment must be a positive power of 2 (otherwise UB). An ubsan checker will need to take that into account as well.

Mon, Dec 17, 10:51 AM

Thu, Dec 13

jyknight accepted D53491: [Sparc] Add membar assembler tags.
Thu, Dec 13, 7:16 AM

Wed, Dec 12

jyknight added a comment to D54355: Use is.constant intrinsic for __builtin_constant_p.

That example cannot be expected to ever evaluate the expression as "1" -- it doesn't in GCC, nor should it in Clang. An asm constraint of "n" or "i" (but not, e.g., "nr") must require a constant expression, and evaluating the argument as a constant expression necessarily means always evaluating it to -1.

Wed, Dec 12, 7:54 PM
jyknight added a comment to D55616: Emit ASM input in a constant context.

This seems like a good start, but not complete.

Wed, Dec 12, 7:50 PM
jyknight added a comment to D54355: Use is.constant intrinsic for __builtin_constant_p.

The issue is that "n" is expecting an immediate value, but the result of the ternary operator isn't calculated by the front-end, because it doesn't "know" that the evaluation of __builtin_constant_p shouldn't be delayed (it being compiled at -O0). I suspect that this issue will also happen with the "i" constraint.

Wed, Dec 12, 2:05 PM

Tue, Dec 11

jyknight added a comment to D54355: Use is.constant intrinsic for __builtin_constant_p.

Here's the test case that we have https://reviews.llvm.org/P8123 gcc seems to accept it at O0

Tue, Dec 11, 10:57 AM

Thu, Dec 6

jyknight added a comment to D54814: Move internal usages of `alignof`/`__alignof` to use `_LIBCPP_ALIGNOF`. .

I'd note that most of this diff was not actually required to fix the issue. E.g, in the locations where an alignment is passed to allocation/deallocation functions directly, it is perfectly fine to use the __alignof value instead of the alignof value.

Thu, Dec 6, 3:01 PM
jyknight added a comment to D55150: Emit warnings from the driver for use of -mllvm or -Xclang options..

I'm not sure that putting a warning that can be disabled really helps here; anyone who needs the option will just disable the warning anyway, and then users adding additional options somewhere else in the build system will miss the warning.

Instead, it would probably be better to rename Xclang and mllvm to something that makes it clear the user is doing something unsupported. Maybe "--unstable-llvm-option" and "--unstable-clang-option" or something like that. (This will lead to some breakage, but the breakage is roughly equivalent for anyone using -Werror.)

Thu, Dec 6, 12:08 PM
jyknight added a comment to D55150: Emit warnings from the driver for use of -mllvm or -Xclang options..

Using -Xclang is the only way to pass options to the static analyzer, I don't think we should warn on it.

Thu, Dec 6, 11:17 AM
jyknight added a comment to D55150: Emit warnings from the driver for use of -mllvm or -Xclang options..

There is a cost to having people encode these flags into their build systems -- it can then cause issues if we ever change these internal flags. I do not think any Clang maintainer intends to support these as stable APIs, unlike most of the driver command-line. But, neither -Xclang nor -mllvm obviously scream out "don't use this!", and so people may then add them to their buildsystems without causing reviewers to say "Wait...really? Are you sure that's a good idea?".

This is why I proposed a compromise, allow this warning to be disabled completely for people actively using those flags, which are pretty much exclusively toolchain developers (well basically what I proposed, since it's not clear what counts as a build made by someone who is working and debugging a pass, being fully aware of those flags, using the subset of them specific to that pass/feature, I would say assertion+dump enabled builds are closest, but having an explicit build flag for it would be nicer). It's more unified than everyone either adding workarounds into build systems or disabling it completely (by just removing it).

Thu, Dec 6, 10:47 AM
jyknight added a comment to D55150: Emit warnings from the driver for use of -mllvm or -Xclang options..

Personally I'm against this type of warning as it's likely anyone using -mllvm is actually intending to adjust certain behaviors across one or more passes with a lot of switches supported by it being intentionally hidden from <llvm_tool> --help output requiring explicitly specifying that hidden flags be shown.

Thu, Dec 6, 9:32 AM
jyknight added inline comments to D52416: Allow FP types for atomicrmw xchg.
Thu, Dec 6, 8:40 AM
jyknight added a comment to D53965: IR: Add fp operations to atomicrmw.

I'd note that this change isn't a necessary prerequisite to implementing the C++ feature -- FP math is already available in C11, and clang already implements it by lowering to cmpxchg. I'd be very skeptical of this addition, if you hadn't mentioned that AMDGPU supported it in hardware. AFAIK, there's no other ISAs that support atomic fadd in hardware. However, that AMDGPU does have hardware support seems like a sufficient reason to add support for it to LLVM.

Thu, Dec 6, 8:27 AM

Wed, Dec 5

jyknight added a comment to D55150: Emit warnings from the driver for use of -mllvm or -Xclang options..

I think this should be internal-driver-option and the text updated? I don't think these are necessarily experimental, they're internals of the compiler implementation, and not a supported interface for users. Unsure how to best explain this.

Wed, Dec 5, 12:08 PM
jyknight added a comment to D52416: Allow FP types for atomicrmw xchg.

Looks reasonable.

Wed, Dec 5, 11:46 AM
jyknight added a comment to D54653: Remove branch from CreateAlignmentAssumption.

Thus, I think that the simple expression will still work even with this use case.

Wed, Dec 5, 9:29 AM

Tue, Dec 4

jyknight added a comment to D53491: [Sparc] Add membar assembler tags.

Probably worth adding a range check on the membar operand when it's specified as an integer that it's actually in the range 0 <= val <= 127.

Tue, Dec 4, 3:28 PM
jyknight accepted D51614: [Sparc] Use float register for integer constrained with "f" in inline asm.

Seems odd, but this appears to match GCC's behavior, so, LG.

Tue, Dec 4, 3:06 PM
jyknight added inline comments to D54653: Remove branch from CreateAlignmentAssumption.
Tue, Dec 4, 1:22 PM
jyknight added inline comments to D54653: Remove branch from CreateAlignmentAssumption.
Tue, Dec 4, 9:06 AM

Mon, Dec 3

jyknight accepted D54547: PTH-- Remove feature entirely-.

Since you've already submitted a fix to Boost, https://github.com/boostorg/build/commit/3385fe2aa699a45e722a1013658f824b6a7c761f, I think this is fine to remove.

Mon, Dec 3, 3:07 PM
jyknight closed D54807: llvm-git: More tweaks..

Forgot to close with the commit, r347766.

Mon, Dec 3, 2:38 PM
jyknight added inline comments to D55057: [Headers] Make max_align_t match GCC's implementation..
Mon, Dec 3, 2:36 PM
jyknight added inline comments to D54653: Remove branch from CreateAlignmentAssumption.
Mon, Dec 3, 1:55 PM

Fri, Nov 30

jyknight created D55150: Emit warnings from the driver for use of -mllvm or -Xclang options..
Fri, Nov 30, 3:13 PM

Thu, Nov 29

jyknight added inline comments to D55057: [Headers] Make max_align_t match GCC's implementation..
Thu, Nov 29, 3:07 PM
jyknight added inline comments to D54653: Remove branch from CreateAlignmentAssumption.
Thu, Nov 29, 1:12 PM
jyknight added inline comments to D55057: [Headers] Make max_align_t match GCC's implementation..
Thu, Nov 29, 1:05 PM
jyknight updated subscribers of D55057: [Headers] Make max_align_t match GCC's implementation..
Thu, Nov 29, 1:03 PM
jyknight accepted D48131: [RISCV] Implement codegen for cmpxchg on RV32IA.

Looks good, ship it.

Thu, Nov 29, 11:53 AM
jyknight committed rL347883: git-llvm: Fix incremental population of svn tree..
git-llvm: Fix incremental population of svn tree.
Thu, Nov 29, 8:49 AM

Wed, Nov 28

jyknight committed rL347766: llvm-git: More tweaks..
llvm-git: More tweaks.
Wed, Nov 28, 7:33 AM

Wed, Nov 21

jyknight added a comment to D54807: llvm-git: More tweaks..

BTW, I've tested this on Linux in both python2 and python3, but not on windows.

Wed, Nov 21, 12:38 PM
jyknight created D54807: llvm-git: More tweaks..
Wed, Nov 21, 12:02 PM

Nov 16 2018

jyknight added a comment to D54341: Speed up git-llvm script by only svn up'ing affected directories..

Hopefully resolved the python3 compatibility with r347113.

Nov 16 2018, 4:04 PM
jyknight committed rL347113: Make git-llvm python3 compatible again. Hopefully. :).
Make git-llvm python3 compatible again. Hopefully. :)
Nov 16 2018, 4:02 PM
jyknight committed rL347103: Speed up git-llvm script by only svn up'ing affected directories..
Speed up git-llvm script by only svn up'ing affected directories.
Nov 16 2018, 2:39 PM
jyknight closed D54341: Speed up git-llvm script by only svn up'ing affected directories..
Nov 16 2018, 2:38 PM

Nov 9 2018

jyknight created D54341: Speed up git-llvm script by only svn up'ing affected directories..
Nov 9 2018, 11:56 AM
jyknight committed rL346550: Branch/tag all projects with a single commit in release-tagging script..
Branch/tag all projects with a single commit in release-tagging script.
Nov 9 2018, 11:47 AM
jyknight closed D53467: Branch/tag all projects with a single commit in release-tagging script..
Nov 9 2018, 11:47 AM

Nov 7 2018

jyknight committed rL346334: Workaround PPC backend bug in test for r346322..
Workaround PPC backend bug in test for r346322.
Nov 7 2018, 9:04 AM
jyknight committed rL346322: Add support for llvm.is.constant intrinsic (PR4898).
Add support for llvm.is.constant intrinsic (PR4898)
Nov 7 2018, 7:27 AM
jyknight closed D4276: Added llvm.is.constant intrinsic.
Nov 7 2018, 7:26 AM · deleted

Nov 6 2018

jyknight added a comment to D4276: Added llvm.is.constant intrinsic.

The SCCP bug is already be exhibited by the tests in test/CodeGen/Generic/is-constant.ll which pass a struct.

I'd prefer a testcase that explicitly calls "opt -sccp", as opposed to implicitly relying on the fact that opt -O2 includes SCCP. (You can just add a small test to test/Transforms/SCCP/ipsccp-basic.ll .)

Nov 6 2018, 3:10 PM · deleted
jyknight updated the diff for D4276: Added llvm.is.constant intrinsic.

Added sccp testcase.

Nov 6 2018, 3:10 PM · deleted
jyknight added a comment to D53414: Add instructions for migrating branches from one git repository to another..

Let's submit this soon, even if it will be further edited.

Nov 6 2018, 2:28 PM
jyknight added a comment to D4276: Added llvm.is.constant intrinsic.

Picking this back up again...

Nov 6 2018, 1:51 PM · deleted
jyknight updated the diff for D4276: Added llvm.is.constant intrinsic.

Rebased onto current head, no changes.

Nov 6 2018, 1:49 PM · deleted
jyknight updated the diff for D53467: Branch/tag all projects with a single commit in release-tagging script..

Also remove and recreate the branch in a single commit if using
--rebranch.

Nov 6 2018, 1:28 PM

Nov 1 2018

jyknight added inline comments to D53877: [IR] Strawman for dedicated FNeg IR instruction.
Nov 1 2018, 11:26 AM

Oct 30 2018

jyknight added inline comments to D53852: [IR] Allow increasing the alignment of dso-local globals..
Oct 30 2018, 1:21 PM

Oct 26 2018

jyknight added inline comments to D53332: Access ADDLIB in llvm-ar via command line.
Oct 26 2018, 11:05 AM

Oct 19 2018

jyknight created D53467: Branch/tag all projects with a single commit in release-tagging script..
Oct 19 2018, 6:33 PM

Oct 18 2018

jyknight added a comment to D52968: [TI removal] Update the C API for the move away from `TerminatorInst`..

LGTM.

Oct 18 2018, 3:55 PM
jyknight accepted D52968: [TI removal] Update the C API for the move away from `TerminatorInst`..
Oct 18 2018, 3:53 PM
jyknight added inline comments to D52968: [TI removal] Update the C API for the move away from `TerminatorInst`..
Oct 18 2018, 3:32 PM

Oct 14 2018

jyknight added inline comments to D52939: ExprConstant: Propagate correct return values from constant evaluation..
Oct 14 2018, 7:54 PM
jyknight updated the diff for D52939: ExprConstant: Propagate correct return values from constant evaluation..

Address most comments.

Oct 14 2018, 7:54 PM

Oct 12 2018

jyknight created D53199: Fix the behavior of clang's -w flag..
Oct 12 2018, 8:42 AM

Oct 10 2018

jyknight committed rL344183: llvm-ar: Darwin archive format fixes..
llvm-ar: Darwin archive format fixes.
Oct 10 2018, 2:10 PM

Oct 9 2018

jyknight committed rL344110: ExprConstant: Make __builtin_object_size use EM_IgnoreSideEffects..
ExprConstant: Make __builtin_object_size use EM_IgnoreSideEffects.
Oct 9 2018, 7:56 PM
jyknight committed rC344110: ExprConstant: Make __builtin_object_size use EM_IgnoreSideEffects..
ExprConstant: Make __builtin_object_size use EM_IgnoreSideEffects.
Oct 9 2018, 7:56 PM
jyknight closed D52924: Make __builtin_object_size use the EM_IgnoreSideEffects evaluation mode..
Oct 9 2018, 7:56 PM
jyknight added inline comments to D50694: [Sparc] Give the option to use the OS reserved global registers.
Oct 9 2018, 8:22 AM
jyknight accepted D52703: Allow ifunc resolvers to accept arguments.

Given that there's no technical reason for the compiler to prohibit this (it was just clang trying to diagnose a probable user-error, which turns out to not be as probable as ), this seems like the right solution to me.

Oct 9 2018, 7:57 AM

Oct 5 2018

jyknight committed rL343892: Emit CK_NoOp casts in C mode, not just C++..
Emit CK_NoOp casts in C mode, not just C++.
Oct 5 2018, 2:55 PM
jyknight committed rC343892: Emit CK_NoOp casts in C mode, not just C++..
Emit CK_NoOp casts in C mode, not just C++.
Oct 5 2018, 2:55 PM
jyknight closed D52918: Emit CK_NoOp casts in C mode, not just C++..
Oct 5 2018, 2:55 PM
jyknight closed D52918: Emit CK_NoOp casts in C mode, not just C++..
Oct 5 2018, 2:55 PM
jyknight added a child revision for D52924: Make __builtin_object_size use the EM_IgnoreSideEffects evaluation mode.: D52939: ExprConstant: Propagate correct return values from constant evaluation..
Oct 5 2018, 11:09 AM
jyknight added parent revisions for D52939: ExprConstant: Propagate correct return values from constant evaluation.: D52924: Make __builtin_object_size use the EM_IgnoreSideEffects evaluation mode., D52926: Rename EM_ConstantExpressionUnevaluated to EM_ConstantFoldUnevaluated, which is actually what the code is currently doing., D52918: Emit CK_NoOp casts in C mode, not just C++..
Oct 5 2018, 11:09 AM
jyknight added a child revision for D52926: Rename EM_ConstantExpressionUnevaluated to EM_ConstantFoldUnevaluated, which is actually what the code is currently doing.: D52939: ExprConstant: Propagate correct return values from constant evaluation..
Oct 5 2018, 11:09 AM
jyknight added a child revision for D52918: Emit CK_NoOp casts in C mode, not just C++.: D52939: ExprConstant: Propagate correct return values from constant evaluation..
Oct 5 2018, 11:09 AM
jyknight created D52939: ExprConstant: Propagate correct return values from constant evaluation..
Oct 5 2018, 11:07 AM
jyknight committed rL343867: Emit diagnostic note when calling an invalid function declaration..
Emit diagnostic note when calling an invalid function declaration.
Oct 5 2018, 10:52 AM
jyknight committed rC343867: Emit diagnostic note when calling an invalid function declaration..
Emit diagnostic note when calling an invalid function declaration.
Oct 5 2018, 10:52 AM
jyknight closed D52919: Emit diagnostic note when calling an invalid function declaration..
Oct 5 2018, 10:51 AM
jyknight updated the diff for D52918: Emit CK_NoOp casts in C mode, not just C++..

Added test.

Oct 5 2018, 8:06 AM
jyknight added a comment to D52918: Emit CK_NoOp casts in C mode, not just C++..

Patch is missing tests -- perhaps you could dump the AST and check the casting notation from the output?

Oct 5 2018, 8:05 AM

Oct 4 2018

jyknight created D52926: Rename EM_ConstantExpressionUnevaluated to EM_ConstantFoldUnevaluated, which is actually what the code is currently doing..
Oct 4 2018, 10:48 PM
jyknight abandoned D52925: Rename EM_ConstantExpressionUnevaluated to EM_ConstantFoldUnevaluated, which is actually what the code is currently doing..

https://reviews.llvm.org/D52926

Oct 4 2018, 10:48 PM
jyknight created D52925: Rename EM_ConstantExpressionUnevaluated to EM_ConstantFoldUnevaluated, which is actually what the code is currently doing..
Oct 4 2018, 10:37 PM
jyknight created D52924: Make __builtin_object_size use the EM_IgnoreSideEffects evaluation mode..
Oct 4 2018, 10:37 PM
jyknight created D52919: Emit diagnostic note when calling an invalid function declaration..
Oct 4 2018, 5:25 PM
jyknight created D52918: Emit CK_NoOp casts in C mode, not just C++..
Oct 4 2018, 5:20 PM
jyknight committed rL343805: Give same-named members unique timestamps on Darwin in llvm-ar..
Give same-named members unique timestamps on Darwin in llvm-ar.
Oct 4 2018, 11:52 AM
jyknight closed D47659: Give same-named members unique timestamps on Darwin in llvm-ar..
Oct 4 2018, 11:52 AM
jyknight added a comment to D47659: Give same-named members unique timestamps on Darwin in llvm-ar..

Just noticed I forgot to commit this. Will do so now. :)

Oct 4 2018, 11:42 AM

Oct 2 2018

jyknight committed rL343598: Delete /lldb/tmp directory created in r324484..
Delete /lldb/tmp directory created in r324484.
Oct 2 2018, 9:07 AM

Sep 27 2018

jyknight added a comment to D51487: [Sparc] Remove the support for builtin setjmp/longjmp.

LGTM. :)

Sep 27 2018, 12:26 PM

Sep 18 2018

jyknight accepted D52234: [docs][AtomicExpandPass] Document the alternate lowering strategy for part-word atomicrmw/cmpxchg.

I'd prefer it to more strongly indicate that the IR-level LL/SC lowering is broken and should not be used in the future, and hopefully will be removed once targets have migrated off of it.

Sep 18 2018, 10:02 AM
jyknight accepted D48130: [AtomicExpandPass]: Add a hook for custom cmpxchg expansion in IR.

Straightforward enough; LG.

Sep 18 2018, 9:11 AM
jyknight accepted D47882: [RISCV] Codegen for i8, i16, and i32 atomicrmw with RV32A.

Final update looks good as far as I can tell.

Sep 18 2018, 8:58 AM

Aug 29 2018

jyknight added a comment to D50969: [Sparc] Improve the builtin setjmp/longjmp.

There is one user of __builtin_setjmp/__builtin_longjmp that should be kept in mind: Ruby.

Aug 29 2018, 2:59 PM

Aug 28 2018

jyknight added a comment to D50969: [Sparc] Improve the builtin setjmp/longjmp.

A revert won't apply cleanly by now, so probably best to make a review for it. I suspect it's simpler to just delete the code as it is now, rather than try to actually do a git revert.

Aug 28 2018, 12:40 PM

Aug 26 2018

jyknight added a comment to D48638: [Sparc] Add support for the cycle counter available in GR740.

It's possibly only theoretical at the moment, since I don't know if anything both supports this and is little endian, but --
On a little endian system, will the registers be swapped; that is, would ASR22 hold the 32 LSB instead of ASR23?

Aug 26 2018, 10:19 AM
jyknight accepted D50971: [Sparc] Avoid writing outside array in applyFixup.
Aug 26 2018, 9:57 AM
jyknight added a comment to D50969: [Sparc] Improve the builtin setjmp/longjmp.

Dare I ask -- are you fixing them because you're testing with -verify-machineinstrs and fixing the failures, or because you actually want it for something?

Aug 26 2018, 9:36 AM
jyknight accepted D50964: [Sparc] Use ANDN instead of AND if constant can be encoded more efficiently.

Seems unclear to me that the "more than one use" bit is the right assumption. But sure, let's go with that for now.

Aug 26 2018, 9:17 AM