Page MenuHomePhabricator

spatel (Sanjay Patel)
User

Projects

User does not belong to any projects.

User Details

User Since
May 22 2014, 1:24 PM (348 w, 5 d)

Recent Activity

Today

spatel added a comment to D95373: Replace vector intrinsics with call to vector library.

I haven't looked at the trade-offs, but if we can do this transform later ( SelectionDAGLegalize::ConvertNodeToLibcall() ? ) that would seem to be more flexible...and smaller patch?
For reference:
D8131 - added support for external vector library calls
D70107 - added inject-tli-mappings

+1 on doing this as late as possible. Ideally we would then just get rid of the logic in LV/SLP that inserts the library calls.

I think SelectionDAG would scalarize the operation in LegalizeVectorOps before they reach LegalizeDAG.

Tue, Jan 26, 11:26 AM · Restricted Project
spatel accepted D95455: [InstCombine] Preserve FMF for powi simplifications..

LGTM

Tue, Jan 26, 11:21 AM · Restricted Project
spatel accepted D95454: [NFC] Show instcombine powi simplifications drop FMF..

Thanks - LGTM.

Tue, Jan 26, 11:18 AM · Restricted Project
spatel added inline comments to D95351: NFC: Migrate SimplifyCFG to work on InstructionCost.
Tue, Jan 26, 10:49 AM · Restricted Project
spatel added a comment to D95454: [NFC] Show instcombine powi simplifications drop FMF..

Use something other than fast on the 2nd call to provide better test coverage.
Please update the CHECK lines with utils/update_test_checks.py (use "--function=powi" if you want to avoid diffs on other functions).

Tue, Jan 26, 10:40 AM · Restricted Project
spatel requested review of D95452: [LoopVectorize] use IR fast-math-flags exclusively (not function attributes).
Tue, Jan 26, 9:13 AM · Restricted Project
spatel committed rG00773ef78aaa: [LoopVectorize] add test for fmin/fmax FMF propagation; NFC (authored by spatel).
[LoopVectorize] add test for fmin/fmax FMF propagation; NFC
Tue, Jan 26, 8:24 AM
spatel committed rG09b1c56366b5: [LoopUtils] do not initialize Cmp predicate unnecessarily; NFC (authored by spatel).
[LoopUtils] do not initialize Cmp predicate unnecessarily; NFC
Tue, Jan 26, 8:24 AM
spatel added a comment to D95373: Replace vector intrinsics with call to vector library.

I haven't looked at the trade-offs, but if we can do this transform later ( SelectionDAGLegalize::ConvertNodeToLibcall() ? ) that would seem to be more flexible...and smaller patch?
For reference:
D8131 - added support for external vector library calls
D70107 - added inject-tli-mappings

Tue, Jan 26, 5:32 AM · Restricted Project

Yesterday

spatel committed rG09a136bcc694: [InstCombine] narrow min/max intrinsics with extended inputs (authored by spatel).
[InstCombine] narrow min/max intrinsics with extended inputs
Mon, Jan 25, 4:58 AM
spatel committed rG07b60d006068: [InstCombine] add tests for min/max intrinsics with extended values; NFC (authored by spatel).
[InstCombine] add tests for min/max intrinsics with extended values; NFC
Mon, Jan 25, 4:58 AM

Sun, Jan 24

spatel committed rG77adbe6a8c71: [SLP] fix fast-math requirements for fmin/fmax reductions (authored by spatel).
[SLP] fix fast-math requirements for fmin/fmax reductions
Sun, Jan 24, 6:11 AM

Sat, Jan 23

spatel committed rGa6f02212764a: [SLP] fix fast-math-flag propagation on FP reductions (authored by spatel).
[SLP] fix fast-math-flag propagation on FP reductions
Sat, Jan 23, 8:32 AM
spatel committed rG39e1e53a7c16: [SLP] add reduction test with mixed fast-math-flags; NFC (authored by spatel).
[SLP] add reduction test with mixed fast-math-flags; NFC
Sat, Jan 23, 8:32 AM

Fri, Jan 22

spatel committed rG411c144e4c99: [InstCombine] narrow abs with sign-extended input (authored by spatel).
[InstCombine] narrow abs with sign-extended input
Fri, Jan 22, 10:45 AM
spatel committed rGe679eea6d20d: [InstCombine] add tests for abs(sext X); NFC (authored by spatel).
[InstCombine] add tests for abs(sext X); NFC
Fri, Jan 22, 10:44 AM
spatel closed D95235: [InstCombine] narrow abs with sign-extended input.
Fri, Jan 22, 10:44 AM · Restricted Project
spatel added a comment to D95235: [InstCombine] narrow abs with sign-extended input.

Cheers - please can you add the PR48816 test case

define i8 @transform_abs_epi8(i8 %0) {
  %2 = sext i8 %0 to i32
  %3 = tail call i32 @llvm.abs.i32(i32 %2, i1 true)
  %4 = trunc i32 %3 to i8
  ret i8 %4
}
declare i32 @llvm.abs.i32(i32, i1 immarg)

(and vector equivalent)

Fri, Jan 22, 10:35 AM · Restricted Project
spatel requested review of D95235: [InstCombine] narrow abs with sign-extended input.
Fri, Jan 22, 7:16 AM · Restricted Project
spatel committed rGa4914dc1f2b4: [SLP] do not traverse constant uses (authored by willir).
[SLP] do not traverse constant uses
Fri, Jan 22, 5:14 AM
spatel closed D94713: Do not traverse ConstantData use-list in SLPVectorizer.
Fri, Jan 22, 5:14 AM · Restricted Project

Thu, Jan 21

spatel committed rG2f03528f5e7f: [SLP] rename reduction variable to avoid shadowing; NFC (authored by spatel).
[SLP] rename reduction variable to avoid shadowing; NFC
Thu, Jan 21, 1:03 PM
spatel committed rGd77753381fe0: [SLP] simplify reduction matching (authored by spatel).
[SLP] simplify reduction matching
Thu, Jan 21, 12:06 PM
spatel added inline comments to D94480: [NFC] [DAGCombine] Correct the result for sqrt even the iteration is zero.
Thu, Jan 21, 8:07 AM · Restricted Project
spatel committed rG070af1b7887f: [InstCombine] avoid crashing on attribute propagation (authored by spatel).
[InstCombine] avoid crashing on attribute propagation
Thu, Jan 21, 5:13 AM
spatel closed D95088: [InstCombine] avoid crashing on attribute propagation.
Thu, Jan 21, 5:13 AM · Restricted Project
spatel added inline comments to D95088: [InstCombine] avoid crashing on attribute propagation.
Thu, Jan 21, 4:54 AM · Restricted Project

Wed, Jan 20

spatel requested review of D95088: [InstCombine] avoid crashing on attribute propagation.
Wed, Jan 20, 1:46 PM · Restricted Project
spatel added a comment to D93476: [LV][ARM] Inloop reduction cost modelling.

The cost model part of the patch LGTM. I think that someone more familiar with the loop vectorizer should have a look at that part.

Wed, Jan 20, 11:57 AM · Restricted Project
spatel committed rGc09be0d2a0f9: [SLP] reduce reduction code for checking vectorizable ops; NFC (authored by spatel).
[SLP] reduce reduction code for checking vectorizable ops; NFC
Wed, Jan 20, 8:15 AM
spatel committed rG1c54112a5762: [SLP] refactor more reduction functions; NFC (authored by spatel).
[SLP] refactor more reduction functions; NFC
Wed, Jan 20, 8:15 AM
spatel committed rG8590d245434d: [SLP] move reduction createOp functions; NFC (authored by spatel).
[SLP] move reduction createOp functions; NFC
Wed, Jan 20, 8:15 AM

Tue, Jan 19

spatel added a reviewer for D93129: [LV] Do not use vector type to compute cost of scalar address comp.: pengfei.

I haven't looked at x86 gather/scatter code at all - ping @pengfei for more thoughts on that.

Tue, Jan 19, 9:13 AM · Restricted Project
spatel added a comment to D93476: [LV][ARM] Inloop reduction cost modelling.

I'm not up-to-date with the current state of MVE and other Arm ISA additions, so others should feel free to chime in. :)
Instead of adding a specialization parameter for MLA to normal math reductions, would it be simpler/cleaner to add a dedicated cost model API for MLA? For example, we already distinguish min/max from other ops via getMinMaxReductionCost().

Tue, Jan 19, 5:23 AM · Restricted Project
spatel accepted D94713: Do not traverse ConstantData use-list in SLPVectorizer.

I have not followed the plans for changes to the IR, etc. But this doesn't seem to conflict with any optimizations that SLP tries to do, so LGTM.

Tue, Jan 19, 4:57 AM · Restricted Project

Mon, Jan 18

spatel committed rGd27bb5c375ca: [x86] add cast to avoid compile-time warning; NFC (authored by spatel).
[x86] add cast to avoid compile-time warning; NFC
Mon, Jan 18, 2:47 PM
spatel committed rG5b77ac32b115: [SLP] match maxnum/minnum intrinsics as FP reduction ops (authored by spatel).
[SLP] match maxnum/minnum intrinsics as FP reduction ops
Mon, Jan 18, 2:37 PM
spatel closed D94913: [SLP] match maxnum/minnum intrinsics as FP reduction ops.
Mon, Jan 18, 2:37 PM · Restricted Project
spatel updated the diff for D94913: [SLP] match maxnum/minnum intrinsics as FP reduction ops.

Patch updated:

  1. Change cost calc to use getMinMaxReductionCost() (this is copied from the existing integer min/max code).
  2. Added test coverage/comments to demonstrate FMF constraints.
Mon, Jan 18, 9:39 AM · Restricted Project
spatel committed rGca7e27054c25: [SLP] add more FMF tests for fmax/fmin reductions; NFC (authored by spatel).
[SLP] add more FMF tests for fmax/fmin reductions; NFC
Mon, Jan 18, 9:25 AM
spatel added inline comments to D94913: [SLP] match maxnum/minnum intrinsics as FP reduction ops.
Mon, Jan 18, 9:19 AM · Restricted Project
spatel added inline comments to D94713: Do not traverse ConstantData use-list in SLPVectorizer.
Mon, Jan 18, 7:41 AM · Restricted Project
spatel updated the diff for D94913: [SLP] match maxnum/minnum intrinsics as FP reduction ops.

Patch updated: I changed some variable names with d1c4e85 and missed rebasing this diff.

Mon, Jan 18, 7:31 AM · Restricted Project
spatel requested review of D94913: [SLP] match maxnum/minnum intrinsics as FP reduction ops.
Mon, Jan 18, 6:59 AM · Restricted Project
spatel committed rG3dbbadb8ef53: [SLP] rename reduction query for min/max ops; NFC (authored by spatel).
[SLP] rename reduction query for min/max ops; NFC
Mon, Jan 18, 6:39 AM
spatel committed rGd1c4e859ce42: [SLP] reduce opcode API dependency in reduction cost calc; NFC (authored by spatel).
[SLP] reduce opcode API dependency in reduction cost calc; NFC
Mon, Jan 18, 6:39 AM

Sat, Jan 16

spatel committed rG49b96cd9ef2f: [SLP] remove opcode field from reduction data class (authored by spatel).
[SLP] remove opcode field from reduction data class
Sat, Jan 16, 11:11 AM
spatel committed rGfcfcc3cc6b16: [SLP] fix typos; NFC (authored by spatel).
[SLP] fix typos; NFC
Sat, Jan 16, 11:11 AM
spatel committed rG48dbac5b6b0b: [SLP] remove unnecessary use of 'OperationData' (authored by spatel).
[SLP] remove unnecessary use of 'OperationData'
Sat, Jan 16, 11:11 AM

Fri, Jan 15

spatel committed rGceb3cdccd0fb: [SLP] remove dead code in reduction matching; NFC (authored by spatel).
[SLP] remove dead code in reduction matching; NFC
Fri, Jan 15, 2:03 PM
spatel committed rG1f21de535d37: [SLP] remove unused reduction functions; NFC (authored by spatel).
[SLP] remove unused reduction functions; NFC
Fri, Jan 15, 11:59 AM

Thu, Jan 14

spatel committed rGb21905dfe379: [SLP] remove unnecessary state in matching reductions (authored by spatel).
[SLP] remove unnecessary state in matching reductions
Thu, Jan 14, 3:32 PM

Wed, Jan 13

spatel committed rG123674a81674: [SLP] simplify type check for reductions (authored by spatel).
[SLP] simplify type check for reductions
Wed, Jan 13, 10:31 AM
spatel added a comment to D94576: [LoopVectorize] Guard verifyFunction with EXPENSIVE_CHECKS macro.
Wed, Jan 13, 8:52 AM · Restricted Project
spatel committed rGe433ca28ec92: [SLP] add reduction test for FMF; NFC (authored by spatel).
[SLP] add reduction test for FMF; NFC
Wed, Jan 13, 8:44 AM

Tue, Jan 12

spatel committed rG9e7895a8682c: [SLP] reduce code duplication while processing reductions; NFC (authored by spatel).
[SLP] reduce code duplication while processing reductions; NFC
Tue, Jan 12, 1:05 PM
spatel committed rG92fb5c49e8aa: [SLP] rename variable to improve readability; NFC (authored by spatel).
[SLP] rename variable to improve readability; NFC
Tue, Jan 12, 1:05 PM
spatel committed rG554be30a4280: [SLP] reduce code duplication in processing reductions; NFC (authored by spatel).
[SLP] reduce code duplication in processing reductions; NFC
Tue, Jan 12, 1:05 PM
spatel committed rG46507a96fc13: [SLP] reduce code duplication while matching reductions; NFC (authored by spatel).
[SLP] reduce code duplication while matching reductions; NFC
Tue, Jan 12, 1:05 PM

Mon, Jan 11

spatel committed rG288f3fc5dfee: [InstCombine] reduce icmp(ashr X, C1), C2 to sign-bit test (authored by spatel).
[InstCombine] reduce icmp(ashr X, C1), C2 to sign-bit test
Mon, Jan 11, 12:54 PM
spatel closed D94014: [InstCombine] reduce icmp(ashr X, C1), C2 to sign-bit test.
Mon, Jan 11, 12:53 PM · Restricted Project
spatel added a comment to D94433: [ValueTracking] Check that alignment is non-zero in computeKnownBitsFromAssume (PR48713)..

Please upload patch with context:
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

Mon, Jan 11, 11:04 AM · Restricted Project
spatel added reviewers for D94433: [ValueTracking] Check that alignment is non-zero in computeKnownBitsFromAssume (PR48713).: scui, jdoerfert.

Adding more potential reviewers based on D88669 (where this code was added).

Mon, Jan 11, 11:02 AM · Restricted Project
spatel added a comment to D94014: [InstCombine] reduce icmp(ashr X, C1), C2 to sign-bit test.

Another way is to use assume to encode the precondition if you prefer: https://alive2.llvm.org/ce/z/__szVL

Mon, Jan 11, 9:37 AM · Restricted Project
spatel added a comment to D94014: [InstCombine] reduce icmp(ashr X, C1), C2 to sign-bit test.

Anyone see problems with this Alive2 implementation using count-leading-*?
https://alive2.llvm.org/ce/z/SWxadd

Mon, Jan 11, 8:09 AM · Restricted Project
spatel updated subscribers of D94014: [InstCombine] reduce icmp(ashr X, C1), C2 to sign-bit test.

alive1 does not actually have a countLeadingOnes() precondition

Mon, Jan 11, 7:18 AM · Restricted Project
spatel updated the diff for D94014: [InstCombine] reduce icmp(ashr X, C1), C2 to sign-bit test.

Ping.
Note: I added a bitwidth > 2 constraint to be safe and avoid degenerate narrow bit patterns, but I'm not sure how to write negative tests for that would actually survive to this point in instcombine (instsimplify appears to reduce those first).

Mon, Jan 11, 6:14 AM · Restricted Project

Sun, Jan 10

spatel committed rG3f09c77d33dc: [SLP] fix typo in assert (authored by spatel).
[SLP] fix typo in assert
Sun, Jan 10, 10:16 AM
spatel committed rG0aa75fb12faa: [SLP] put verifyFunction call behind EXPENSIVE_CHECKS (authored by spatel).
[SLP] put verifyFunction call behind EXPENSIVE_CHECKS
Sun, Jan 10, 10:05 AM
spatel closed D94328: [SLP] put verifyFunction call behind EXPENSIVE_CHECKS.
Sun, Jan 10, 10:05 AM · Restricted Project

Fri, Jan 8

spatel requested review of D94328: [SLP] put verifyFunction call behind EXPENSIVE_CHECKS.
Fri, Jan 8, 11:30 AM · Restricted Project
spatel committed rG267ff7901c74: [SLP] limit verifyFunction to debug build (PR48689) (authored by spatel).
[SLP] limit verifyFunction to debug build (PR48689)
Fri, Jan 8, 5:10 AM

Thu, Jan 7

spatel committed rG4c7148d75cd7: [SLP] remove opcode identifier for reduction; NFC (authored by spatel).
[SLP] remove opcode identifier for reduction; NFC
Thu, Jan 7, 11:07 AM
spatel updated subscribers of D89697: * [x86] Implement smarter instruction lowering for FP_TO_UINT from vXf32 to vXi32 for SSE2 and AVX2 by using the exact semantic of the CVTTPS2SI instruction..

No regression appeared in our internal testcases.
It seems the transform is correct, have you verified it with alive-tv?

Thu, Jan 7, 10:34 AM · Restricted Project

Wed, Jan 6

spatel committed rG4c022b5a41de: [SLP] use reduction kind's opcode to create new instructions; NFC (authored by spatel).
[SLP] use reduction kind's opcode to create new instructions; NFC
Wed, Jan 6, 11:38 AM
spatel committed rG5d24089a7001: [SLP] reduce code for propagating flags on reductions; NFC (authored by spatel).
[SLP] reduce code for propagating flags on reductions; NFC
Wed, Jan 6, 11:38 AM
spatel accepted D93599: [DAG] Simplify OR(X,SHL(Y,BW/2)) eq/ne 0/-1 'all/any-of' style patterns.

LGTM - see inline for possible missed comment.

Wed, Jan 6, 10:37 AM · Restricted Project
spatel added inline comments to D93599: [DAG] Simplify OR(X,SHL(Y,BW/2)) eq/ne 0/-1 'all/any-of' style patterns.
Wed, Jan 6, 9:03 AM · Restricted Project
spatel accepted D93891: [DAGCombine] Remove the check for unsafe-fp-math when we are checking the AFN.

Hmm, so we can try to remove those parts that not relative with the parameter arsenm mentioned. I guess most of the check in the DAGCombiner could be removed if I understand correctly. To remove it completely, some work is needed for parameters or others that current IR cannot represent the semantics of the global setting. Is it right ?

Maybe, we can start it with this simple patch first ?

Wed, Jan 6, 8:01 AM · Restricted Project
spatel added a comment to D94069: [NFC][InstructionCost]Migrate VectorCombine.cpp to use InstructionCost.

I haven't followed the details enough to comment on the changes directly, but thanks for the cleanup! The mismatched signed/unsigned cost model APIs are/were a mess.

Wed, Jan 6, 7:42 AM · Restricted Project

Tue, Jan 5

spatel committed rG6a03f8ab629b: [SLP] reduce code for finding reduction costs; NFC (authored by spatel).
[SLP] reduce code for finding reduction costs; NFC
Tue, Jan 5, 2:36 PM
spatel committed rG5a1d31a28490: [SLP] use reduction kind's opcode for cost model queries; NFC (authored by spatel).
[SLP] use reduction kind's opcode for cost model queries; NFC
Tue, Jan 5, 12:27 PM
spatel committed rGd4a999b453a4: [SLP] reduce code duplication; NFC (authored by spatel).
[SLP] reduce code duplication; NFC
Tue, Jan 5, 12:27 PM
spatel accepted D94056: [CodeGen] Update transformations to use poison for shufflevector/insertelem's initial vector elem.

LGTM - other reviewers are probably more familiar with InterleavedAccess than me, so might want to wait for a 2nd opinion.
I made another assert suggestion for that code. Similar to the assert that you have proposed in this patch, we could add the asserts before this patch just to make the shuffle assumptions clearer (assuming those are correct assertions).

Tue, Jan 5, 11:32 AM · Restricted Project
spatel committed rG3b8b2c7da2ef: [SLP] delete unused pairwise reduction option (authored by spatel).
[SLP] delete unused pairwise reduction option
Tue, Jan 5, 10:39 AM
spatel closed D93860: [SLP] delete unused pairwise reduction option.
Tue, Jan 5, 10:39 AM · Restricted Project
spatel added a comment to D93599: [DAG] Simplify OR(X,SHL(Y,BW/2)) eq/ne 0/-1 'all/any-of' style patterns.

Does this work with scalars too? Let me know if I missed the test(s).

Tue, Jan 5, 9:07 AM · Restricted Project

Mon, Jan 4

spatel committed rG36263a7cccc0: [LoopUtils] remove redundant opcode parameter; NFC (authored by spatel).
[LoopUtils] remove redundant opcode parameter; NFC
Mon, Jan 4, 2:05 PM
spatel committed rG976695752416: [LoopUtils] reduce code for creatng reduction; NFC (authored by spatel).
[LoopUtils] reduce code for creatng reduction; NFC
Mon, Jan 4, 1:05 PM
spatel committed rG58b6c5d932a0: [LoopUtils] reorder logic for creating reduction; NFC (authored by spatel).
[LoopUtils] reorder logic for creating reduction; NFC
Mon, Jan 4, 1:05 PM
spatel committed rG6976812129bf: [InstCombine] add tests for ashr+icmp; NFC (authored by spatel).
[InstCombine] add tests for ashr+icmp; NFC
Mon, Jan 4, 10:35 AM
spatel updated the diff for D94014: [InstCombine] reduce icmp(ashr X, C1), C2 to sign-bit test.

Patch updated:
Fixed semi-colon typos in test comments.

Mon, Jan 4, 10:14 AM · Restricted Project
spatel added inline comments to D94014: [InstCombine] reduce icmp(ashr X, C1), C2 to sign-bit test.
Mon, Jan 4, 10:14 AM · Restricted Project
spatel requested review of D94014: [InstCombine] reduce icmp(ashr X, C1), C2 to sign-bit test.
Mon, Jan 4, 9:13 AM · Restricted Project

Fri, Jan 1

spatel committed rGc74e8539ff37: [Analysis] flatten enums for recurrence types (authored by spatel).
[Analysis] flatten enums for recurrence types
Fri, Jan 1, 9:46 AM
spatel committed rGc182a000954d: [Analysis] fix typo in code comment; NFC (authored by spatel).
[Analysis] fix typo in code comment; NFC
Fri, Jan 1, 9:45 AM

Thu, Dec 31

spatel committed rGeaab71106b81: [Analysis] reduce code for matching min/max; NFC (authored by spatel).
[Analysis] reduce code for matching min/max; NFC
Thu, Dec 31, 2:20 PM
spatel added a comment to D93860: [SLP] delete unused pairwise reduction option.

Yep, that's definitely wrong. But I think FMF propagation is not an easy thing. We cannot always copy the original FMF, right? E.g. reassoc reduction may result in inf which "sequential" doesn't. Should ninf be propagated or not after reduction?

Thu, Dec 31, 7:02 AM · Restricted Project
spatel accepted D92015: [DAGCombiner] Fold BRCOND(FREEZE(COND)) to BRCOND(COND).

LGTM - please pre-commit the test with the current (trunk) codegen, so we see the diff (and don't lose the test in case we have to revert).

Thu, Dec 31, 6:20 AM · Restricted Project

Wed, Dec 30

spatel committed rG8ca60db40bd9: [LoopUtils] reduce FMF and min/max complexity when forming reductions (authored by spatel).
[LoopUtils] reduce FMF and min/max complexity when forming reductions
Wed, Dec 30, 12:31 PM