Page MenuHomePhabricator

spatel (Sanjay Patel)
User

Projects

User does not belong to any projects.

User Details

User Since
May 22 2014, 1:24 PM (392 w, 2 d)

Recent Activity

Yesterday

spatel committed rG53b00b821582: [InstSimplify] Fold X {lshr,udiv} C <u X --> true for nonzero X, non-identity C (authored by erikdesjardins).
[InstSimplify] Fold X {lshr,udiv} C <u X --> true for nonzero X, non-identity C
Fri, Nov 26, 1:49 PM
spatel closed D114279: [InstSimplify] Fold X {lshr,udiv} C <u X --> true for nonzero X, non-identity C.
Fri, Nov 26, 1:49 PM · Restricted Project
spatel committed rGa68af62b422d: [InstSimplify] baseline tests for icmp of lshr/udiv fold (NFC) (authored by erikdesjardins).
[InstSimplify] baseline tests for icmp of lshr/udiv fold (NFC)
Fri, Nov 26, 12:57 PM
spatel closed D114280: [InstSimplify] precommit tests for icmp of lshr/udiv fold (NFC).
Fri, Nov 26, 12:57 PM · Restricted Project

Wed, Nov 24

spatel added inline comments to D114339: [InstCombine] simplify (~A | B) ^ A --> ~( A & B).
Wed, Nov 24, 11:50 AM · Restricted Project
spatel added a comment to D113970: [SelectionDAG] Add pattern to haveNoCommonBitsSet..

Do we expect all vector types/targets to bypass this in some way? If yes, it's worth adding a comment to the patch description and/or code. If no, can we find a test that shows an asm difference?

What exactly do you mean by bypass it? This addition to haveNoCommonBitsSet works for vector types as well. Do you mean add another lit test for vector types?

Wed, Nov 24, 8:16 AM · Restricted Project
spatel added inline comments to D114339: [InstCombine] simplify (~A | B) ^ A --> ~( A & B).
Wed, Nov 24, 7:47 AM · Restricted Project
spatel accepted D114279: [InstSimplify] Fold X {lshr,udiv} C <u X --> true for nonzero X, non-identity C.

LGTM

Wed, Nov 24, 6:12 AM · Restricted Project
spatel accepted D112754: X86: Fold masked merge pattern when and-not is not available.

LGTM

Wed, Nov 24, 5:55 AM · Restricted Project
spatel added a comment to D113970: [SelectionDAG] Add pattern to haveNoCommonBitsSet..

Do we expect all vector types/targets to bypass this in some way? If yes, it's worth adding a comment to the patch description and/or code. If no, can we find a test that shows an asm difference?

Wed, Nov 24, 5:49 AM · Restricted Project
spatel added a comment to D113216: [InstCombine] (~(a | b) & c) | ~(a | b | c) -> ~(a | b).

As discussed in D114462, I noticed a pair of missing 'xor' simplifications based on these patterns, so I added those with:
892648b18a8c
b326c058146f

Wed, Nov 24, 5:37 AM · Restricted Project
spatel committed rGb326c058146f: [InstSimplify] fold xor logic of 2 variables, part 2 (authored by spatel).
[InstSimplify] fold xor logic of 2 variables, part 2
Wed, Nov 24, 5:16 AM
spatel committed rG823fc8aa0681: [InstSimplify] add tests for xor logic; NFC (authored by spatel).
[InstSimplify] add tests for xor logic; NFC
Wed, Nov 24, 5:16 AM

Tue, Nov 23

spatel committed rG892648b18a8c: [InstSimplify] fold xor logic of 2 variables (authored by spatel).
[InstSimplify] fold xor logic of 2 variables
Tue, Nov 23, 1:52 PM
spatel closed D114462: [InstSimplify] fold xor logic of 2 variables.
Tue, Nov 23, 1:51 PM · Restricted Project
spatel added a comment to D114462: [InstSimplify] fold xor logic of 2 variables.

LGTM.

Thanks Sanjay! This really helps D113216 with -O3, but then not a more complex version of it which needs demorganing and reassociation:

; (~a & ~b & c) | ~(a | (b | c)) -> ~(a | b)
; (~a & ~b & c) | ~(b | (a | c)) -> ~(a | b)

define i32 @not_and_not_and_or_not_or_or(i32 %a, i32 %b, i32 %c) {
; CHECK-LABEL: @not_and_not_and_or_not_or_or(
; CHECK-NEXT:    [[OR3:%.*]] = or i32 [[B:%.*]], [[A:%.*]]
; CHECK-NEXT:    [[NOT2:%.*]] = xor i32 [[OR3]], -1
; CHECK-NEXT:    ret i32 [[NOT2]]
;
  %or1 = or i32 %b, %c
  %or2 = or i32 %or1, %a
  %not1 = xor i32 %or2, -1
  %nota = xor i32 %a, -1
  %notb = xor i32 %b, -1
  %and1 = and i32 %nota, %notb
  %and2 = and i32 %and1, %c
  %or4 = or i32 %and2, %not1
  call void @use(i32 %nota)
  call void @use(i32 %notb)
  ret i32 %or4
}

declare void @use(i32)

In this case reassociation is blocked by uses, so I was looking into extending LHS match in D113216 to catch (~a & ~b & c) in addition to (~(a | b) & c). I still need to look into that, but this is clearly LGTM.

Tue, Nov 23, 12:46 PM · Restricted Project
spatel committed rG14d743457c3d: [InstSimplify] add tests for xor logic fold; NFC (authored by spatel).
[InstSimplify] add tests for xor logic fold; NFC
Tue, Nov 23, 12:37 PM
spatel added a comment to D114462: [InstSimplify] fold xor logic of 2 variables.

Note: we miss the sibling with swapped and/or too, and it doesn't need a 'not' op, so it would clearly be best placed in instsimplify:
https://alive2.llvm.org/ce/z/7LbWeV

Tue, Nov 23, 11:13 AM · Restricted Project
spatel requested review of D114462: [InstSimplify] fold xor logic of 2 variables.
Tue, Nov 23, 11:03 AM · Restricted Project
spatel committed rG430ad9697d14: [InstCombine] enhance bitwise select matching (authored by spatel).
[InstCombine] enhance bitwise select matching
Tue, Nov 23, 7:05 AM
spatel committed rGe6cd157407a2: [InstCombine] add tests for logical select; NFC (authored by spatel).
[InstCombine] add tests for logical select; NFC
Tue, Nov 23, 7:05 AM

Mon, Nov 22

spatel requested review of D114386: [InstCombine] use decomposeBitTestICmp to make icmp (trunc X), C more consistent.
Mon, Nov 22, 12:38 PM · Restricted Project
spatel accepted D113179: [Passes] Move AggressiveInstCombine after InstCombine.

any more comments? @lebedev.ri @spatel

Mon, Nov 22, 12:06 PM · Restricted Project
spatel committed rGcbb75129b7cf: [InstCombine] regenerate test checks; NFC (authored by spatel).
[InstCombine] regenerate test checks; NFC
Mon, Nov 22, 11:44 AM
spatel committed rG78dc50e5a1a3: [InstCombine] avoid 'tmp' usage in test files; NFC (authored by spatel).
[InstCombine] avoid 'tmp' usage in test files; NFC
Mon, Nov 22, 11:44 AM
spatel committed rG8bfcf1ab6c6d: [InstCombine] move/add tests for binops with sext operand; NFC (authored by spatel).
[InstCombine] move/add tests for binops with sext operand; NFC
Mon, Nov 22, 11:44 AM
spatel added inline comments to D114279: [InstSimplify] Fold X {lshr,udiv} C <u X --> true for nonzero X, non-identity C.
Mon, Nov 22, 10:51 AM · Restricted Project
spatel updated subscribers of D114339: [InstCombine] simplify (~A | B) ^ A --> ~( A & B).
  1. Please commit the baseline tests, so we see diffs here.
  2. I expect that will show that the commuted variations are not testing what they claim to be testing (you'll need to add instructions to prevent commuting).
  3. Please add tests with extra uses - since we're creating 2 instructions, the fold should be good as long as any one of the intermediate values can be eliminated (has one use).
Mon, Nov 22, 8:09 AM · Restricted Project
spatel accepted D113442: [InstCombine] Enable fold select into operand for FAdd, FMul, FSub and FDiv..

You wrote that fdiv ran overnight without completing. I wonder if that opcode is slower than the others. For example, did fadd/fsub also timeout?

Mon, Nov 22, 6:56 AM · Restricted Project

Sun, Nov 21

spatel accepted D112955: [InstCombine] (~(a | b) & c) | ~(c | (a ^ b)) -> ~((a | b) & (c | (b ^ a))).

LGTM

Sun, Nov 21, 7:49 AM · Restricted Project

Sat, Nov 20

spatel committed rG337948ac6e22: [InstCombine] add folds for binop with sexted bool and constant operands (authored by spatel).
[InstCombine] add folds for binop with sexted bool and constant operands
Sat, Nov 20, 9:34 AM
spatel committed rG1d007d0e5a92: [InstCombine] add tests for bitwise logic with bool op; NFC (authored by spatel).
[InstCombine] add tests for bitwise logic with bool op; NFC
Sat, Nov 20, 9:33 AM

Fri, Nov 19

spatel committed rG491efa7f31cb: [InstCombine] add/adjust tests for mask of sext i1; NFC (authored by spatel).
[InstCombine] add/adjust tests for mask of sext i1; NFC
Fri, Nov 19, 1:07 PM
spatel added inline comments to D112955: [InstCombine] (~(a | b) & c) | ~(c | (a ^ b)) -> ~((a | b) & (c | (b ^ a))).
Fri, Nov 19, 4:56 AM · Restricted Project

Wed, Nov 17

spatel added reviewers for D112955: [InstCombine] (~(a | b) & c) | ~(c | (a ^ b)) -> ~((a | b) & (c | (b ^ a))): foad, lebedev.ri.

Double-check that I'm getting the pattern right, but I don't think the 'or' side of this should have 6 instructions. There has to be a smarter way (!), but I worked this out on paper with a truth table:
https://alive2.llvm.org/ce/z/BP3ZZm

Wed, Nov 17, 8:20 AM · Restricted Project
spatel accepted D113526: [InstCombine] Generalize complex OR patterns to AND.

LGTM

Wed, Nov 17, 5:33 AM · Restricted Project

Tue, Nov 16

spatel committed rG8fce94f91610: [InstCombine] canonicalize icmp with trunc op into mask and cmp, part 2 (authored by spatel).
[InstCombine] canonicalize icmp with trunc op into mask and cmp, part 2
Tue, Nov 16, 6:29 AM
spatel closed D112634: [InstCombine] canonicalize icmp with trunc op into mask and cmp, part 2.
Tue, Nov 16, 6:29 AM · Restricted Project

Mon, Nov 15

spatel added inline comments to D112634: [InstCombine] canonicalize icmp with trunc op into mask and cmp, part 2.
Mon, Nov 15, 12:25 PM · Restricted Project
spatel updated the summary of D112634: [InstCombine] canonicalize icmp with trunc op into mask and cmp, part 2.
Mon, Nov 15, 12:16 PM · Restricted Project
spatel added a comment to D112634: [InstCombine] canonicalize icmp with trunc op into mask and cmp, part 2.

Ping.
As I mentioned, I'd prefer not to try to add folds and invert an existing fold in one patch. I did add more instcombine tests for a follow-up patch that would use decomposeBitTestICmp.

Mon, Nov 15, 11:39 AM · Restricted Project
spatel accepted D113508: [PatternMatch] Add m_BinOp/m_c_BinOp with specific opcode.

LGTM - there may be more potential uses in functions like InstCombinerImpl::SimplifyUsingDistributiveLaws() or other reassociation folds.

Mon, Nov 15, 11:00 AM · Restricted Project
spatel committed rG7daa95c8fac2: [InstCombine] Fold (A^B)|~A-->~(A&B) (authored by MehrHeidar).
[InstCombine] Fold (A^B)|~A-->~(A&B)
Mon, Nov 15, 9:31 AM
spatel closed D113783: [InstCombine] Fold (A^B)|~A-->~(A&B).
Mon, Nov 15, 9:30 AM · Restricted Project
spatel accepted D113783: [InstCombine] Fold (A^B)|~A-->~(A&B).

Do you have enough commits/reviews now to request commit access? ( https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access )

Mon, Nov 15, 8:40 AM · Restricted Project
spatel added a comment to D26149: [DAGCombiner] Match load by bytes idiom and fold it into a single load.

Did you consider doing the load combine at IR level? I'm now trying to combine loads at IR level during the InstCombine process, so I'm wondering why you didn't do that.

Mon, Nov 15, 8:36 AM · Restricted Project
spatel committed rG3d01507c2dec: [x86] fold vector (X > -1) & Y to shift+andn (2nd try) (authored by spatel).
[x86] fold vector (X > -1) & Y to shift+andn (2nd try)
Mon, Nov 15, 8:11 AM
spatel committed rG6efe64cf9f11: [x86] add test for vector signbit mask fold (PR52504); NFC (authored by spatel).
[x86] add test for vector signbit mask fold (PR52504); NFC
Mon, Nov 15, 8:11 AM
spatel added a comment to D113603: [x86] fold vector (X > -1) & Y to shift+andn.

creduce finished, posted the reduced input here: https://bugs.chromium.org/p/chromium/issues/detail?id=1270222#c5

Mon, Nov 15, 8:04 AM · Restricted Project
spatel added a comment to D113603: [x86] fold vector (X > -1) & Y to shift+andn.

This caused assertions in Chromium builds targeting Windows. See https://bugs.chromium.org/p/chromium/issues/detail?id=1270222#c1 for a reproducer.

I've reverted in 5be64d416481c59dba5faae5e8b8a6fecb578082 until it can be fixed.

I think https://bugs.llvm.org/show_bug.cgi?id=52504 is the same issue

Mon, Nov 15, 5:39 AM · Restricted Project

Sun, Nov 14

spatel committed rGb69dc2d18042: [InstCombine] add tests for or-xor logic fold; NFC (authored by MehrHeidar).
[InstCombine] add tests for or-xor logic fold; NFC
Sun, Nov 14, 8:21 AM
spatel closed D113846: [InstCombine] Pre-commit tests with baseline results .
Sun, Nov 14, 8:20 AM · Restricted Project
spatel accepted D113846: [InstCombine] Pre-commit tests with baseline results .

LGTM

Sun, Nov 14, 8:20 AM · Restricted Project
spatel committed rG254c5246e920: [DAGCombiner] match inverted/swapped patterns for vselect of mask of signbit (authored by spatel).
[DAGCombiner] match inverted/swapped patterns for vselect of mask of signbit
Sun, Nov 14, 6:48 AM
spatel added inline comments to D113783: [InstCombine] Fold (A^B)|~A-->~(A&B).
Sun, Nov 14, 6:06 AM · Restricted Project

Fri, Nov 12

spatel added a reviewer for D113783: [InstCombine] Fold (A^B)|~A-->~(A&B): rampitec.
Fri, Nov 12, 1:09 PM · Restricted Project
spatel committed rG6c32dd4dfafe: [AArch64][x86] add tests for swapped cmp+vselect patterns; NFC (authored by spatel).
[AArch64][x86] add tests for swapped cmp+vselect patterns; NFC
Fri, Nov 12, 12:50 PM
spatel added inline comments to D73575: [InstCombine] canonicalize splat shuffle after cmp.
Fri, Nov 12, 9:13 AM · Restricted Project
spatel accepted D113056: [IVDescriptor] Make sure the sign is included for negative extension..

LGTM

Fri, Nov 12, 8:22 AM · Restricted Project
spatel committed rGbf5748a1af0d: [x86] fold vector (X > -1) & Y to shift+andn (authored by spatel).
[x86] fold vector (X > -1) & Y to shift+andn
Fri, Nov 12, 5:18 AM
spatel closed D113603: [x86] fold vector (X > -1) & Y to shift+andn.
Fri, Nov 12, 5:18 AM · Restricted Project

Thu, Nov 11

spatel added inline comments to D73575: [InstCombine] canonicalize splat shuffle after cmp.
Thu, Nov 11, 11:51 AM · Restricted Project
spatel updated the diff for D113603: [x86] fold vector (X > -1) & Y to shift+andn.

Patch updated:
Added tests with i1 types and runs to exercise AVX512 targets. I'm not very familiar with those patterns, but it looks like that could use some follow-ups? Pre-AVX512 looks ideal to me.

Thu, Nov 11, 11:25 AM · Restricted Project
spatel committed rGce89335fe8c9: [x86] add tests and RUNs for vector compares; NFC (authored by spatel).
[x86] add tests and RUNs for vector compares; NFC
Thu, Nov 11, 11:21 AM
spatel updated the diff for D113603: [x86] fold vector (X > -1) & Y to shift+andn.

Minor update from previous rev - use "X" and "Y" names for captured values since that matches the code comment.

Thu, Nov 11, 9:38 AM · Restricted Project
spatel updated the diff for D113603: [x86] fold vector (X > -1) & Y to shift+andn.

Patch updated:

  1. Spell out "BitWidth" in code comment and patch description for less confusion.
  2. Use local SDValue names for matching to avoid swap risk in subsequent folds.
  3. Create x86 shift node directly.
  4. Rebase after 11522cfcad6b / D113426 - so all of the results in the "vselect-zero.ll" file are ideal now if I'm seeing it properly.
Thu, Nov 11, 9:32 AM · Restricted Project
spatel added inline comments to D113603: [x86] fold vector (X > -1) & Y to shift+andn.
Thu, Nov 11, 9:24 AM · Restricted Project
spatel updated subscribers of D113442: [InstCombine] Enable fold select into operand for FAdd, FMul, FSub and FDiv..
Thu, Nov 11, 8:48 AM · Restricted Project
spatel committed rG11522cfcad6b: [DAGCombiner] add fold for vselect based on mask of signbit, part 3 (authored by spatel).
[DAGCombiner] add fold for vselect based on mask of signbit, part 3
Thu, Nov 11, 7:34 AM
spatel closed D113426: [DAGCombiner] add fold for vselect based on mask of signbit, part 3.
Thu, Nov 11, 7:34 AM · Restricted Project
spatel added inline comments to D113426: [DAGCombiner] add fold for vselect based on mask of signbit, part 3.
Thu, Nov 11, 6:03 AM · Restricted Project

Wed, Nov 10

spatel requested review of D113603: [x86] fold vector (X > -1) & Y to shift+andn.
Wed, Nov 10, 12:50 PM · Restricted Project
spatel added inline comments to D113426: [DAGCombiner] add fold for vselect based on mask of signbit, part 3.
Wed, Nov 10, 12:32 PM · Restricted Project
spatel committed rGa8abd19b1073: [x86] simplify code; NFC (authored by spatel).
[x86] simplify code; NFC
Wed, Nov 10, 10:30 AM
spatel committed rG5424fb164a0f: [x86] fix formatting; NFC (authored by spatel).
[x86] fix formatting; NFC
Wed, Nov 10, 10:30 AM
spatel accepted D113510: [InstCombine] Strip offset when folding and/or of icmps.

LGTM

Wed, Nov 10, 9:46 AM · Restricted Project
spatel accepted D113141: [InstCombine] Relax and reorganize one use checks in the ~(a | b) & c.

LGTM

Wed, Nov 10, 8:37 AM · Restricted Project
spatel added a comment to D113442: [InstCombine] Enable fold select into operand for FAdd, FMul, FSub and FDiv..

I have some concern when I checked with alive2, for fadd https://alive2.llvm.org/ce/z/UjAMM_ alive2 complaints mis-matched outputs.

Wed, Nov 10, 8:22 AM · Restricted Project
spatel added a comment to D113510: [InstCombine] Strip offset when folding and/or of icmps.

I don't see any test diffs with 2 signed preds. Is there any value in adding a test like that to further exercise the range logic?

Wed, Nov 10, 7:58 AM · Restricted Project
spatel added inline comments to D113035: [InstCombine] enhance vector bitwise select matching.
Wed, Nov 10, 7:08 AM · Restricted Project
spatel committed rG67299aa84f50: [InstCombine] add check for integer source type from cast to prevent crash (authored by spatel).
[InstCombine] add check for integer source type from cast to prevent crash
Wed, Nov 10, 6:51 AM
spatel committed rG51baafd23822: [x86] add tests for signbit splat mask patterns; NFC (authored by spatel).
[x86] add tests for signbit splat mask patterns; NFC
Wed, Nov 10, 6:51 AM
spatel committed rGbe9e892e9ddc: [x86] shorten function name; NFC (authored by spatel).
[x86] shorten function name; NFC
Wed, Nov 10, 6:51 AM
spatel added inline comments to D113035: [InstCombine] enhance vector bitwise select matching.
Wed, Nov 10, 6:00 AM · Restricted Project

Tue, Nov 9

spatel added inline comments to D113141: [InstCombine] Relax and reorganize one use checks in the ~(a | b) & c.
Tue, Nov 9, 1:05 PM · Restricted Project
spatel committed rGd5c002bdc735: [InstCombine] fix code comment to match code; NFC (authored by spatel).
[InstCombine] fix code comment to match code; NFC
Tue, Nov 9, 11:27 AM
spatel committed rG2a88d00cf250: [InstCombine] fold sub-of-umax to 0-usubsat (authored by spatel).
[InstCombine] fold sub-of-umax to 0-usubsat
Tue, Nov 9, 9:46 AM
spatel committed rGde12ca31d477: [InstCombine] fix typo in test; NFC (authored by spatel).
[InstCombine] fix typo in test; NFC
Tue, Nov 9, 9:46 AM
spatel accepted D113132: [InstCombine] Fuse checks for LHS (~(A | B) & C) | ... NFC..

With all that said it looks like these patterns have to be extracted into a separate function and called for both 'or' and 'and'. But I don't think this shall preclude this NFC and the next D113141 to wait for it, these are mostly orthogonal to me.

Tue, Nov 9, 9:29 AM · Restricted Project
spatel committed rGbaa6a851308d: [InstCombine] allow commute in sub-of-umax fold (authored by spatel).
[InstCombine] allow commute in sub-of-umax fold
Tue, Nov 9, 8:05 AM
spatel committed rGad48fc35e2dc: [InstCombine] add/move tests for sub-of-umax; NFC (authored by spatel).
[InstCombine] add/move tests for sub-of-umax; NFC
Tue, Nov 9, 8:05 AM
spatel committed rGc36b7e21bd8f: [InstCombine] enhance vector bitwise select matching (authored by spatel).
[InstCombine] enhance vector bitwise select matching
Tue, Nov 9, 5:55 AM
spatel closed D113035: [InstCombine] enhance vector bitwise select matching.
Tue, Nov 9, 5:55 AM · Restricted Project
spatel added inline comments to D113442: [InstCombine] Enable fold select into operand for FAdd, FMul, FSub and FDiv..
Tue, Nov 9, 5:50 AM · Restricted Project
spatel added inline comments to D113442: [InstCombine] Enable fold select into operand for FAdd, FMul, FSub and FDiv..
Tue, Nov 9, 5:44 AM · Restricted Project
spatel updated the diff for D113035: [InstCombine] enhance vector bitwise select matching.

Patch updated:
Use general VectorType APIs to make the transform safe for scalable vectors. It is not possible to write a test for this currently -- and surrounding code would likely crash -- but this should work with matchers (m_Not) that are made more flexible in the future.

Tue, Nov 9, 5:03 AM · Restricted Project
spatel added inline comments to D113035: [InstCombine] enhance vector bitwise select matching.
Tue, Nov 9, 4:58 AM · Restricted Project

Mon, Nov 8

spatel requested review of D113426: [DAGCombiner] add fold for vselect based on mask of signbit, part 3.
Mon, Nov 8, 12:30 PM · Restricted Project
spatel committed rGe2b1d3260a30: [AArch][x86] add tests for vselect; NFC (authored by spatel).
[AArch][x86] add tests for vselect; NFC
Mon, Nov 8, 12:21 PM
spatel updated subscribers of D113035: [InstCombine] enhance vector bitwise select matching.
Mon, Nov 8, 12:16 PM · Restricted Project