Page MenuHomePhabricator

nikic (Nikita Popov)
User

Projects

User does not belong to any projects.

User Details

User Since
May 5 2018, 9:37 AM (153 w, 2 d)

Recent Activity

Yesterday

nikic updated the diff for D100264: [SCEV] Don't walk uses of phis without SCEV expression when forgetting.

Add const qualifier.

Mon, Apr 12, 2:11 PM · Restricted Project
nikic added a comment to D99773: [InstCombine] when calling conventions are compatible, don't convert the call to undef idiom.

I've temporarily reverted this change because it causes a significant compile-time regression, see https://llvm-compile-time-tracker.com/compare.php?from=4b7bad9eaea2233521a94f6b096aaa88dc584e23&to=f4d682d6ce6c5b3a41a0acf297507c82f5c21eef&stat=instructions. I assume that wasn't intentional.

Mon, Apr 12, 2:07 PM · Restricted Project
nikic added a reverting change for rGf4d682d6ce6c: [InstCombine] when calling conventions are compatible, don't convert the call…: rGa3fabc79ae9d: Revert "[InstCombine] when calling conventions are compatible, don't convert….
Mon, Apr 12, 1:57 PM
nikic committed rGa3fabc79ae9d: Revert "[InstCombine] when calling conventions are compatible, don't convert… (authored by nikic).
Revert "[InstCombine] when calling conventions are compatible, don't convert…
Mon, Apr 12, 1:57 PM
nikic added a reverting change for D99773: [InstCombine] when calling conventions are compatible, don't convert the call to undef idiom: rGa3fabc79ae9d: Revert "[InstCombine] when calling conventions are compatible, don't convert….
Mon, Apr 12, 1:57 PM · Restricted Project
nikic added inline comments to D100264: [SCEV] Don't walk uses of phis without SCEV expression when forgetting.
Mon, Apr 12, 12:52 AM · Restricted Project

Sun, Apr 11

nikic added inline comments to D95543: [GVN] Clobber partially aliased loads..
Sun, Apr 11, 2:10 PM · Restricted Project
nikic added a comment to D74640: [LoopRotate] Add explicit flag to require MSSA..

I believe this can be abandoned now, it's no longer relevant now that LICM runs before LoopRotate, so MSSA is required at that point anyway.

Sun, Apr 11, 1:23 PM · Restricted Project
nikic added a comment to D100211: [InstCombine] Fold cmpeq/ne(and(~X,Y),0) --> cmpeq/ne(or(freeze(X),Y),freeze(X)) (PR44136).

The missing middle-end fold was noted on the ticket (PR44136) - this patch is to show what would be necessary.

Limiting this to isGuaranteedNotToBeUndefOrPoison would be safer, although I'm not sure how common the case is tbh. Happy to abandon and close the ticket if this is pointless extra work.

Sun, Apr 11, 11:07 AM · Restricted Project
nikic added a reviewer for D100211: [InstCombine] Fold cmpeq/ne(and(~X,Y),0) --> cmpeq/ne(or(freeze(X),Y),freeze(X)) (PR44136): aqjune.

I'm somewhat doubtful that this fold will be beneficial if it requires inserting freeze. Is there some broader context here for which this is intended?

Sun, Apr 11, 8:34 AM · Restricted Project
nikic requested changes to D100095: [InstCombine] Conditionally emitting nsw/nuw flags when combining two add operations.
Sun, Apr 11, 8:31 AM · Restricted Project
nikic added inline comments to D100226: Explicitly annotate nofree functions inferred from readonly/readnone.
Sun, Apr 11, 8:06 AM · Restricted Project
nikic added a comment to D100179: [GVN][NFC] Refactor code and add description for GVN object.

Just FYI, recently work on NewGVN has started again, so that's where the focus will (hopefully) be in the future.

Sun, Apr 11, 7:57 AM · Restricted Project
nikic accepted D99469: GlobalISel: Restrict narrow scalar for fptoui/fptosi results.

(Just to be sure, I checked that the added tests did fail previously.)

Sun, Apr 11, 7:44 AM · Restricted Project
nikic added inline comments to D99299: Normalize interaction with boolean attributes.
Sun, Apr 11, 7:32 AM · Restricted Project
nikic requested review of D100264: [SCEV] Don't walk uses of phis without SCEV expression when forgetting.
Sun, Apr 11, 7:08 AM · Restricted Project

Sat, Apr 10

nikic committed rG8de2f1ff79aa: [IVUsers] Check LoopSimplify cache earlier (NFC) (authored by nikic).
[IVUsers] Check LoopSimplify cache earlier (NFC)
Sat, Apr 10, 2:00 PM
nikic added a comment to D99469: GlobalISel: Restrict narrow scalar for fptoui/fptosi results.

Reverse ping, I believe @foad's comment regarding the test is still open.

Sat, Apr 10, 3:51 AM · Restricted Project

Fri, Apr 9

nikic added a comment to D99759: [LoopUnroll] avoid assumption clone explosion.

For reference, this is the IR after unrolling but before simplification for the PhaseOrdering test: https://gist.github.com/nikic/efd3aae8b9282902a418f99e01ff5134 (note that mass of assumes on extractvalues is unproblematic, the issue are the assumes on %.pre.pre).

Fri, Apr 9, 1:09 PM · Restricted Project
nikic added a comment to D99759: [LoopUnroll] avoid assumption clone explosion.

Taking your example from https://bugs.llvm.org/show_bug.cgi?id=49785#c2, it seems to be scaling with about O(n^5.5) with iteration count, which is beyond all reasonable bounds. I think this needs to be mitigated at a lower level than unrolling.

Fri, Apr 9, 11:36 AM · Restricted Project
nikic added a comment to D98718: [AA][NFC] Convert AliasResult to class containing offset for PartialAlias case..

Possibly MergeAliasResults should preserve the offset when merging two PartialAlias with the same offset?

Fri, Apr 9, 12:34 AM · Restricted Project

Thu, Apr 8

nikic committed rG59a2f67011ba: [LoopRotate] Don't split loop pass manager (authored by nikic).
[LoopRotate] Don't split loop pass manager
Thu, Apr 8, 1:05 PM
nikic closed D99843: [LoopRotate] Don't split loop pass manager.
Thu, Apr 8, 1:05 PM · Restricted Project
nikic updated the diff for D98152: [InstCombine] Canonicalize SPF to min/max intrinsics.

Rebase

Thu, Apr 8, 12:09 PM · Restricted Project
nikic accepted D99977: [GVN] Properly invalidate ICF cache when we simplify a value.

LGTM

Thu, Apr 8, 11:57 AM · Restricted Project
nikic added reviewers for D100095: [InstCombine] Conditionally emitting nsw/nuw flags when combining two add operations: lebedev.ri, nikic.
Thu, Apr 8, 4:02 AM · Restricted Project
nikic added inline comments to D99977: [GVN] Properly invalidate ICF cache when we simplify a value.
Thu, Apr 8, 12:49 AM · Restricted Project

Wed, Apr 7

nikic added inline comments to D99424: [BasicAA] Be more careful with modulo ops on VariableGEPIndex..
Wed, Apr 7, 3:14 PM · Restricted Project
nikic added inline comments to D99424: [BasicAA] Be more careful with modulo ops on VariableGEPIndex..
Wed, Apr 7, 3:01 PM · Restricted Project
nikic added a comment to D99299: Normalize interaction with boolean attributes.

As you are now asserting that only certain values are allowed, I believe you also need to add a verifier check that checks this. It should not be possible to assert using a valid module.

Wed, Apr 7, 1:39 PM · Restricted Project
nikic accepted D99642: For non-null pointer checks, do not descend through out-of-bounds GEPs.

LGTM, just some suggestions for the test.

Wed, Apr 7, 9:34 AM · Restricted Project
nikic added a comment to D90098: [BasicAA] Don't pass through AA metadata (NFCI).

@jeroen.dobbelaere I'm not sure on the details, but it may be necessary to pass through just the ptr_provenance information with full restrict and include it in the cache key (while omitting all the other metadata).

Wed, Apr 7, 7:19 AM · Restricted Project
nikic added a comment to D99674: [InstCombine] Conditionally fold select i1 into and/or.

@spatel These are important canonicalization transforms, that allow us to treat logical and/or the same way as and/or in many other passes. There is shouldAvoidAbsorbingNotIntoSelect() to guard against infinite loops, and probably it needs to be applied in more places.

Wed, Apr 7, 6:24 AM · Restricted Project

Tue, Apr 6

nikic accepted D99909: [GVN] Add missing ICF update.

LGTM

Tue, Apr 6, 9:41 AM · Restricted Project
nikic accepted D99961: [SimplifyInst] Use correct type for GEPs with vector indices..

It probably makes sense to land this first in any case, so LGTM.

Tue, Apr 6, 9:13 AM · Restricted Project
nikic added a comment to D99961: [SimplifyInst] Use correct type for GEPs with vector indices..

This looks okay, but I wonder whether we shouldn't be adjusting the getIndexedType() API instead. We have similar code elsewhere, for example in https://github.com/llvm/llvm-project/blob/9c5ebf0358960adf28931569a0c801b56c8008d9/llvm/lib/IR/Constants.cpp#L2415-L2430.

Tue, Apr 6, 9:06 AM · Restricted Project
nikic accepted D99581: [test, GVN] Fix use of var defined in CHECK-NOT.

LGTM

Tue, Apr 6, 8:50 AM · Restricted Project
nikic added a comment to D99581: [test, GVN] Fix use of var defined in CHECK-NOT.

I'd suggest to simply switch these to update_tests_checks.py instead. Or is there some reason this is not possible for these tests?

Tue, Apr 6, 3:08 AM · Restricted Project

Mon, Apr 5

nikic added a comment to D99814: [JumpThreading] Change asserts for WantInteger into actual checks.

From someone who isn't very familiar with this code: Why do we need the preference flag in the first place? As indirectbr isn't particularly common, can't we check whether the result is a BlockAddress after the fact?

Mon, Apr 5, 2:16 AM · Restricted Project

Sun, Apr 4

nikic committed rG9bad7de9a3fb: [SimplifyCFG] Handle two equal cases in switch to select (authored by nikic).
[SimplifyCFG] Handle two equal cases in switch to select
Sun, Apr 4, 8:32 AM
nikic committed rG7ca168dd5ada: [SimplifyCFG] Add switch-to-select test with two equal cases (NFC) (authored by nikic).
[SimplifyCFG] Add switch-to-select test with two equal cases (NFC)
Sun, Apr 4, 8:15 AM
nikic committed rGcb4443994e72: [SimplifyCFG] Make test more robust (NFC) (authored by nikic).
[SimplifyCFG] Make test more robust (NFC)
Sun, Apr 4, 8:15 AM
nikic committed rGfd73e4d4b299: [CVP] Add more tests for select with overdefined operand (NFC) (authored by nikic).
[CVP] Add more tests for select with overdefined operand (NFC)
Sun, Apr 4, 4:54 AM
nikic committed rG72e0846ef87d: [LVI] Don't bail on overdefined value in select (authored by nikic).
[LVI] Don't bail on overdefined value in select
Sun, Apr 4, 2:11 AM
nikic committed rG3ac2541b5c31: [CVP] Add test for and of min (NFC) (authored by nikic).
[CVP] Add test for and of min (NFC)
Sun, Apr 4, 2:11 AM
nikic added a comment to D99853: [InstSimplify] Teach isUndefValue to understand const vector with both undef & poison.

This problem isn't really limited to this one function in InstSimplify. Might it make sense to adjust UndefValue to actually represent such constants? We'd have to add a bitmask to it that indicates which values are poison.

Sun, Apr 4, 1:56 AM · Restricted Project

Sat, Apr 3

nikic accepted D99674: [InstCombine] Conditionally fold select i1 into and/or.

If -instcombine-unsafe-select-transform is flipped from true to false, we have 35 LLVM unit test failures. A few of them are unsafe ones so okay to be removed, but the remaining ones need updates in analysis or transformations to reenable them.
To minimize performance regression, what do you think about this workflow?
(1) This patch is landed to fix the miscompilation without significant regressions
(2) The clang noundef patch is enabled by default (I left a comment at its test-only patch)
(3) Flip -instcombine-unsafe-select-transform to false

Enabling the clang noundef flag reduces assembly diffs from LLVM test suite.
Without the noundef flag, the # of different assembly files is 737/5247 when -instcombine-unsafe-select-transform is flipped from true to false.
With the noundef flag, it becomes 632/5247.
It looks big, but about 80% of those are simply from reordering of basic blocks (I randomly picked 5 samples and 4 were such cases).
With this patch (D99674) only, the assembly diff # is already 439/5247, so 632 isn't very big.

Sat, Apr 3, 12:15 PM · Restricted Project
nikic retitled D99843: [LoopRotate] Don't split loop pass manager from [LoopRotate] Don't split pass manager to [LoopRotate] Don't split loop pass manager.
Sat, Apr 3, 12:04 PM · Restricted Project
nikic requested review of D99843: [LoopRotate] Don't split loop pass manager.
Sat, Apr 3, 12:02 PM · Restricted Project
nikic added inline comments to D98114: [Loads] Forward constant vector store to load of first element.
Sat, Apr 3, 9:23 AM · Restricted Project
nikic committed rG665065821e6a: [FastISel] Remove kill tracking (authored by nikic).
[FastISel] Remove kill tracking
Sat, Apr 3, 6:50 AM
nikic closed D98294: [FastISel] Remove kill tracking.
Sat, Apr 3, 6:50 AM · Restricted Project
nikic added inline comments to D98114: [Loads] Forward constant vector store to load of first element.
Sat, Apr 3, 6:43 AM · Restricted Project
nikic committed rG1470f94d71c5: [InstCombine] Add load/store forwarding test with odd size (NFC) (authored by nikic).
[InstCombine] Add load/store forwarding test with odd size (NFC)
Sat, Apr 3, 6:30 AM
nikic committed rGb552e16b0b04: [Loads] Forward constant vector store to load of first element (authored by nikic).
[Loads] Forward constant vector store to load of first element
Sat, Apr 3, 3:11 AM
nikic closed D98114: [Loads] Forward constant vector store to load of first element.
Sat, Apr 3, 3:10 AM · Restricted Project
nikic committed rG9d20eaf9c08c: [BasicAA] Don't store AATags in cache key (NFC) (authored by nikic).
[BasicAA] Don't store AATags in cache key (NFC)
Sat, Apr 3, 2:36 AM
nikic committed rG17b4e5d45631: [BasicAA] Don't pass through AA metadata (NFCI) (authored by nikic).
[BasicAA] Don't pass through AA metadata (NFCI)
Sat, Apr 3, 2:32 AM
nikic closed D90098: [BasicAA] Don't pass through AA metadata (NFCI).
Sat, Apr 3, 2:31 AM · Restricted Project
nikic updated the diff for D98294: [FastISel] Remove kill tracking.

Add release note.

Sat, Apr 3, 2:07 AM · Restricted Project

Fri, Apr 2

nikic committed rG4a3e006830aa: [LVI] Use range metadata on intrinsics (authored by nikic).
[LVI] Use range metadata on intrinsics
Fri, Apr 2, 7:47 AM
nikic committed rG93135091b1f9: [CVP] Add test for !range on intrinsic (NFC) (authored by nikic).
[CVP] Add test for !range on intrinsic (NFC)
Fri, Apr 2, 7:46 AM

Thu, Apr 1

nikic added a comment to D99249: [PassManager] Run additional LICM before LoopRotate.

Here's a reduced version of @lebedev.ri's example:

Thu, Apr 1, 2:20 PM · Restricted Project
nikic added a comment to D95815: [deref-at-point] restrict inference of dereferenceability based on allocsize attribute.

As we don't have any default-enabled nosync inference, doesn't this effectively disable this code entirely (for the default pipeline)?

Thu, Apr 1, 12:56 AM · Restricted Project

Wed, Mar 31

nikic added a comment to D99249: [PassManager] Run additional LICM before LoopRotate.

Most performance results look ok. There are a few that are mixed (+ & -), one regression that's still investigated that so far seems architecture specific and not related to this patch, a couple of noticeable improvements and a few marginal improvements.
Please note I have only tested the new pass manager.
So, green light from my side assuming the other reviewers are also on board with this.

Wed, Mar 31, 11:40 AM · Restricted Project
nikic added a comment to D99674: [InstCombine] Conditionally fold select i1 into and/or.

Unconditionally disabling this transformation affects later pipelines that depend on and/or i1s.
To minimize its impact, this patch conservatively checks whether cond2 is an instruction that
creates a poison or its operand creates a poison.

Wed, Mar 31, 11:20 AM · Restricted Project
nikic added inline comments to D99648: [GVN] Refactor analyzeLoadFromClobberingWrite to simplify code.
Wed, Mar 31, 11:13 AM · Restricted Project
nikic accepted D99671: [ValueTracking] Add with.overflow intrinsics to poison analysis functions.

LGTM

Wed, Mar 31, 10:08 AM · Restricted Project
nikic added a comment to D99660: Use DL.getIndexType() in Value::getPointerAlignment().

It's not really clear why using the index type for this would be correct. The index type is used for GEP index and casting a pointer to the index type doesn't make a whole lot of sense to me. Do you have any LangRef wording or other usages that would show that this is a sensible thing to do?

Wed, Mar 31, 7:26 AM · Restricted Project
nikic added a comment to D99642: For non-null pointer checks, do not descend through out-of-bounds GEPs.

I think this should be using Val->stripInBoundsOffsets() rather than modify the getUnderlyingObject() API.

Wed, Mar 31, 2:44 AM · Restricted Project

Tue, Mar 30

nikic accepted D99564: [ConstantFolding] Fixing addo/subo with undef.

LGTM

Tue, Mar 30, 12:52 AM · Restricted Project

Mon, Mar 29

nikic committed rG7669455df49e: [X86][FastISel] Fix with.overflow eflags clobber (PR49587) (authored by nikic).
[X86][FastISel] Fix with.overflow eflags clobber (PR49587)
Mon, Mar 29, 2:09 PM
nikic closed D98600: [X86][FastISel] Fix with.overflow eflags clobber (PR49587).
Mon, Mar 29, 2:09 PM · Restricted Project

Sun, Mar 28

nikic committed rGce066da81c3e: [BasicAA] Make sure types match in constant offset heuristic (authored by nikic).
[BasicAA] Make sure types match in constant offset heuristic
Sun, Mar 28, 12:38 PM
nikic updated the diff for D98152: [InstCombine] Canonicalize SPF to min/max intrinsics.

Rebase

Sun, Mar 28, 12:03 PM · Restricted Project
nikic added inline comments to D99469: GlobalISel: Restrict narrow scalar for fptoui/fptosi results.
Sun, Mar 28, 8:20 AM · Restricted Project
nikic committed rG3df3f3df4539: [BasicAA] Handle gep with unknown sizes earlier (NFCI) (authored by nikic).
[BasicAA] Handle gep with unknown sizes earlier (NFCI)
Sun, Mar 28, 6:49 AM
nikic added a comment to D98600: [X86][FastISel] Fix with.overflow eflags clobber (PR49587).

For reference, here's how the alternative approach of not using xor for zero initialization would be look like (if you ignore some test update failures): https://gist.github.com/nikic/04876ffe96a5ee2ffb4be102cbbb8d60

Sun, Mar 28, 6:45 AM · Restricted Project

Sat, Mar 27

nikic committed rG9075864b7375: [BasicAA] Refactor linear expression decomposition (authored by nikic).
[BasicAA] Refactor linear expression decomposition
Sat, Mar 27, 3:32 PM
nikic committed rGb981bc30bf1a: [BasicAA] Correct handle implicit sext in decomposition (authored by nikic).
[BasicAA] Correct handle implicit sext in decomposition
Sat, Mar 27, 7:22 AM
nikic committed rG60f3e8fbe44f: [BasicAA] Clarify entry values of GetLinearExpression() (NFC) (authored by nikic).
[BasicAA] Clarify entry values of GetLinearExpression() (NFC)
Sat, Mar 27, 7:22 AM
nikic committed rGad9dad93ff12: [BasicAA] Bail out earlier for invalid shift amount (authored by nikic).
[BasicAA] Bail out earlier for invalid shift amount
Sat, Mar 27, 4:42 AM
nikic committed rG5a5a8088cc8d: [BasicAA] Retain shl nowrap flags in GetLinearExpression() (authored by nikic).
[BasicAA] Retain shl nowrap flags in GetLinearExpression()
Sat, Mar 27, 4:29 AM
nikic accepted D99452: Make FoldBranchToCommonDest poison-safe by default.

LGTM

Sat, Mar 27, 3:02 AM · Restricted Project
nikic added inline comments to D99452: Make FoldBranchToCommonDest poison-safe by default.
Sat, Mar 27, 2:30 AM · Restricted Project

Fri, Mar 26

nikic reopened D93927: [ArgPromotion] Copy !range metadata for loads..

Here is a further reduction:

Fri, Mar 26, 2:04 PM · Restricted Project
nikic committed rG4622648a069a: Revert "[ArgPromotion] Copy additional metadata for loads." (authored by nikic).
Revert "[ArgPromotion] Copy additional metadata for loads."
Fri, Mar 26, 1:35 PM
nikic added a reverting change for rG166620a4f01f: [ArgPromotion] Copy additional metadata for loads.: rG4622648a069a: Revert "[ArgPromotion] Copy additional metadata for loads.".
Fri, Mar 26, 1:35 PM
nikic added a reverting change for D93927: [ArgPromotion] Copy !range metadata for loads.: rG4622648a069a: Revert "[ArgPromotion] Copy additional metadata for loads.".
Fri, Mar 26, 1:35 PM · Restricted Project
nikic committed rGfd7df0cf3873: [ValueTracking] Handle shl pair in isKnownNonEqual() (authored by nikic).
[ValueTracking] Handle shl pair in isKnownNonEqual()
Fri, Mar 26, 12:21 PM
nikic committed rG9666e89d5778: [ValueTracking] Handle shl in isKnownNonEqual() (authored by nikic).
[ValueTracking] Handle shl in isKnownNonEqual()
Fri, Mar 26, 12:21 PM
nikic committed rG5c85c37c87d6: [ValueTracking] Add tests for non equal shifts (NFC) (authored by nikic).
[ValueTracking] Add tests for non equal shifts (NFC)
Fri, Mar 26, 12:21 PM
nikic added a comment to D99424: [BasicAA] Be more careful with modulo ops on VariableGEPIndex..

I'm somewhat confused by the implementation approach in this patch. I think there's really two orthogonal cases: The first is if all operations are non-wrapping, in which case we can optimize for arbitrary modulos. The second is that we can always use power of two factors from the GCD, because arithmetic is over a power of two field.

Fri, Mar 26, 11:52 AM · Restricted Project
nikic committed rGcaf92a8a92ab: [ValueTracking] Handle non-zero shl recurrence (authored by nikic).
[ValueTracking] Handle non-zero shl recurrence
Fri, Mar 26, 10:43 AM
nikic committed rG41234329b423: [ValueTracking] Add tests for non-zero shl recurrences (NFC) (authored by nikic).
[ValueTracking] Add tests for non-zero shl recurrences (NFC)
Fri, Mar 26, 10:43 AM
nikic committed rG938d05b814c7: [ValueTracking] Handle non-zero add/mul recurrences more precisely (authored by nikic).
[ValueTracking] Handle non-zero add/mul recurrences more precisely
Fri, Mar 26, 10:30 AM
nikic committed rGeac2c94bc226: [ValueTracking] Add more non-zero add/mul recurrence tests (NFC) (authored by nikic).
[ValueTracking] Add more non-zero add/mul recurrence tests (NFC)
Fri, Mar 26, 10:30 AM

Thu, Mar 25

nikic accepted D98422: [ValueTracking] Handle two PHIs in isKnownNonEqual().

LGTM, thanks for pulling this through!

Thu, Mar 25, 3:35 PM · Restricted Project
nikic added inline comments to D98422: [ValueTracking] Handle two PHIs in isKnownNonEqual().
Thu, Mar 25, 3:16 PM · Restricted Project
nikic added a comment to D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration.

I'm worried that this comes up in a lot of places. Perhaps rare still, but important cases. The aarch64 example we have is just a matrix multiply, and is 25% slower with all the cascading spills, https://bugs.llvm.org/show_bug.cgi?id=26810 quotes the same. Like I said before though, the option didn't fix some examples of the same thing that we were seeing in ARM, so I'm not sure how reliably better it is.

@aditya_nandakumar @qcolombet any idea when a better fix for that issue might be available?

Could we just make consider-local-interval-cost -O3 only in the meantime? That should alleviate some of the compile time worries, as we have genuine examples of where it is hurting performance.

Thu, Mar 25, 2:39 PM · Restricted Project