spatel (Sanjay Patel)
User

Projects

User does not belong to any projects.

User Details

User Since
May 22 2014, 1:24 PM (213 w, 3 d)

Recent Activity

Today

spatel added a comment to D48466: [X86] Teach "(select (and (x , 0x1) == 0), y, (z | y) ) -> (-(and (x , 0x1)) & z ) | y" to also handle the case where there is no setcc and the and is used directly with the args swapped.

Not sure how to make Phab select the previous version of the patch.
It's showing the patch from rL335433 now, which is not what we want of course.
Did D48508 affect any of the tests or motivation for this one?

Sun, Jun 24, 7:56 AM
spatel closed D48508: [DAGCombiner] eliminate setcc bool math when input is low-bit of some value.

rL335433 is the commit for this review.

Sun, Jun 24, 7:49 AM
spatel reopened D48466: [X86] Teach "(select (and (x , 0x1) == 0), y, (z | y) ) -> (-(and (x , 0x1)) & z ) | y" to also handle the case where there is no setcc and the and is used directly with the args swapped.

Oops - I included the wrong link in the commit message for rL335433; that was meant to correspond to D48508.

Sun, Jun 24, 7:49 AM
spatel committed rL335433: [DAGCombiner] eliminate setcc bool math when input is low-bit of some value.
[DAGCombiner] eliminate setcc bool math when input is low-bit of some value
Sun, Jun 24, 7:44 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Sun, Jun 24, 7:43 AM
spatel added inline comments to D48508: [DAGCombiner] eliminate setcc bool math when input is low-bit of some value.
Sun, Jun 24, 7:39 AM

Yesterday

spatel updated the diff for D48508: [DAGCombiner] eliminate setcc bool math when input is low-bit of some value.

Patch updated - no functional change intended from the previous rev (test diffs are identical):

  1. Refactored to reduce code duplication and improve readability.
  2. Replaced getValueSizeInBits() == 1 with getValueType() == MVT::i1.
Sat, Jun 23, 8:29 AM

Fri, Jun 22

spatel created D48508: [DAGCombiner] eliminate setcc bool math when input is low-bit of some value.
Fri, Jun 22, 3:36 PM
spatel committed rL335396: [x86] add more tests for bit hacking opportunities with setcc; NFC.
[x86] add more tests for bit hacking opportunities with setcc; NFC
Fri, Jun 22, 3:12 PM
spatel committed rL335395: [PowerPC] add more tests for bit hacking opportunities with setcc; NFC.
[PowerPC] add more tests for bit hacking opportunities with setcc; NFC
Fri, Jun 22, 3:11 PM
spatel added a comment to D48466: [X86] Teach "(select (and (x , 0x1) == 0), y, (z | y) ) -> (-(and (x , 0x1)) & z ) | y" to also handle the case where there is no setcc and the and is used directly with the args swapped.

I was working on a generic DAGCombiner patch for what I think are the early patterns in the motivating case:
https://rise4fun.com/Alive/Gsyp

Fri, Jun 22, 2:53 PM
spatel committed rL335391: [x86] add tests for bit hacking opportunities with setcc; NFC.
[x86] add tests for bit hacking opportunities with setcc; NFC
Fri, Jun 22, 2:21 PM
spatel committed rL335390: [PowerPC] add tests for bit hacking opportunities with setcc; NFC.
[PowerPC] add tests for bit hacking opportunities with setcc; NFC
Fri, Jun 22, 2:21 PM
spatel added a comment to D48467: [X86] Recognize an fnma in the presence of an intervening shuffle..

No, this doesn't deal with this. Should I start from the negate and push it
down if it is transitively used by fma?

Fri, Jun 22, 10:48 AM
spatel added inline comments to D48485: [InstCombine] allow shl+mul combos with shuffle (select) fold (PR37806).
Fri, Jun 22, 10:13 AM
spatel added a comment to D48467: [X86] Recognize an fnma in the presence of an intervening shuffle..

Does this patch deal with the case if we negate the scalar before doing the splat?

Fri, Jun 22, 9:38 AM
spatel added inline comments to D48485: [InstCombine] allow shl+mul combos with shuffle (select) fold (PR37806).
Fri, Jun 22, 9:22 AM
spatel requested review of D48485: [InstCombine] allow shl+mul combos with shuffle (select) fold (PR37806).
Fri, Jun 22, 7:19 AM
spatel updated the diff for D48485: [InstCombine] allow shl+mul combos with shuffle (select) fold (PR37806).

Patch updated:
In the last rev, I forgot to remove the use of the original opcode when we create the new binop.
So we could have shifts when we should have multiplies (and the tests showed that).

Fri, Jun 22, 7:18 AM
spatel planned changes to D48485: [InstCombine] allow shl+mul combos with shuffle (select) fold (PR37806).
Fri, Jun 22, 7:11 AM
spatel accepted D48485: [InstCombine] allow shl+mul combos with shuffle (select) fold (PR37806).

Oops - just noticed a typo that makes this patch wrong.

Fri, Jun 22, 7:11 AM
spatel created D48485: [InstCombine] allow shl+mul combos with shuffle (select) fold (PR37806).
Fri, Jun 22, 7:04 AM
spatel committed rL335347: [InstCombine] add shuffle+binops test from PR37806; NFC.
[InstCombine] add shuffle+binops test from PR37806; NFC
Fri, Jun 22, 6:49 AM
spatel committed rL335345: [InstCombine] add tests for shuffle-with-different-binops; NFC.
[InstCombine] add tests for shuffle-with-different-binops; NFC
Fri, Jun 22, 6:24 AM
spatel committed rL335343: [InstCombine] rearrange shuffle-of-binops logic; NFC.
[InstCombine] rearrange shuffle-of-binops logic; NFC
Fri, Jun 22, 5:50 AM

Thu, Jun 21

spatel committed rL335312: [InstCombine] fix shuffle-of-binops bug.
[InstCombine] fix shuffle-of-binops bug
Thu, Jun 21, 5:01 PM
spatel committed rL335311: [InstCombine] add test for shuffle-of-binops; NFC.
[InstCombine] add test for shuffle-of-binops; NFC
Thu, Jun 21, 4:57 PM
spatel committed rL335301: [IR] fix typo in comment; NFC.
[IR] fix typo in comment; NFC
Thu, Jun 21, 3:32 PM
spatel committed rL335283: [InstCombine] fold vector select of binops with constant ops to 1 binop….
[InstCombine] fold vector select of binops with constant ops to 1 binop…
Thu, Jun 21, 1:19 PM
spatel closed D48401: [InstCombine] fold vector select of binops with constant ops to 1 binop (PR37806).
Thu, Jun 21, 1:19 PM
spatel committed rL335266: [InstCombine] add tests for shuffled cmps; NFC.
[InstCombine] add tests for shuffled cmps; NFC
Thu, Jun 21, 11:12 AM
spatel committed rL335262: [InstCombine] use constant pattern matchers with icmp+sext.
[InstCombine] use constant pattern matchers with icmp+sext
Thu, Jun 21, 10:56 AM
spatel committed rL335261: [InstCombine] add vector icmp tests with undefs; NFC.
[InstCombine] add vector icmp tests with undefs; NFC
Thu, Jun 21, 10:41 AM
spatel committed rL335258: [InstCombine] simplify binops before trying other folds.
[InstCombine] simplify binops before trying other folds
Thu, Jun 21, 10:11 AM
spatel committed rL335257: [LoopVectorize] regenerate full checks; NFC.
[LoopVectorize] regenerate full checks; NFC
Thu, Jun 21, 9:59 AM
spatel updated the diff for D48401: [InstCombine] fold vector select of binops with constant ops to 1 binop (PR37806).

Patch updated:
Committed the helper function change with rL335242, so this is only the shuffle fold now.

Thu, Jun 21, 8:08 AM
spatel committed rL335242: [InstCombine] make div/rem vector constant utility function; NFCI.
[InstCombine] make div/rem vector constant utility function; NFCI
Thu, Jun 21, 8:04 AM
spatel accepted D48301: DAG combine "and|or (select c, -1, 0), x" -> "select c, x, 0|-1".

LGTM.

Thu, Jun 21, 7:39 AM
spatel added reviewers for D48369: [CodeGen] Make block removal order deterministic in CodeGenPrepare: fhahn, bkramer, nhaehnle.

Adding potential reviewers based on a grep of 'deterministic' in recent commits messages. :)

Thu, Jun 21, 7:32 AM
spatel updated the diff for D48401: [InstCombine] fold vector select of binops with constant ops to 1 binop (PR37806).

Patch updated:
No functional change from the previous rev, but use early exits to minimize indents and add TODO comments for the planned enhancements.

Thu, Jun 21, 5:41 AM
spatel added inline comments to D48401: [InstCombine] fold vector select of binops with constant ops to 1 binop (PR37806).
Thu, Jun 21, 5:25 AM

Wed, Jun 20

spatel added inline comments to D48301: DAG combine "and|or (select c, -1, 0), x" -> "select c, x, 0|-1".
Wed, Jun 20, 4:40 PM
spatel created D48401: [InstCombine] fold vector select of binops with constant ops to 1 binop (PR37806).
Wed, Jun 20, 3:51 PM
spatel committed rL335165: [InstCombine] fix typo in test comment; NFC.
[InstCombine] fix typo in test comment; NFC
Wed, Jun 20, 1:21 PM
spatel committed rL335157: [IR] add/use isIntDivRem convenience function.
[IR] add/use isIntDivRem convenience function
Wed, Jun 20, 12:06 PM
spatel accepted D48223: Allow binop C1, (select cc, CF, CT) -> select folding.

LGTM.

Wed, Jun 20, 11:33 AM
spatel committed rL335151: [InstCombine] add vector select of binops tests (PR37806).
[InstCombine] add vector select of binops tests (PR37806)
Wed, Jun 20, 10:53 AM
spatel committed rL335129: [InstSimplify] Fix missed optimization in simplifyUnsignedRangeCheck().
[InstSimplify] Fix missed optimization in simplifyUnsignedRangeCheck()
Wed, Jun 20, 7:27 AM
spatel closed D47922: [InstSimplify] Fix some missed optimization opportunities in simplifyUnsignedRangeCheck().
Wed, Jun 20, 7:27 AM
spatel committed rL335128: [InstSimplify] Add tests for missed optimizations in simplifyUnsignedRangeCheck….
[InstSimplify] Add tests for missed optimizations in simplifyUnsignedRangeCheck…
Wed, Jun 20, 7:07 AM
spatel closed D48000: [InstSimplify][NFC] Add tests for some missed optimization opportunities in simplifyUnsignedRangeCheck().
Wed, Jun 20, 7:07 AM
spatel committed rL335121: [InstCombine] ignore debuginfo when removing redundant assumes (PR37726).
[InstCombine] ignore debuginfo when removing redundant assumes (PR37726)
Wed, Jun 20, 6:27 AM

Tue, Jun 19

spatel added a comment to D48085: [DAGCombiner] restrict (float)((int) f) --> ftrunc with no-signed-zeros.

Should this adjust the ReleaseNotes?

The docs provide extra warning/workaround, and this change doesn't affect that language IMO (there's less chance we're going to break code, but we don't have to disclose that).

For reference, here are links to the docs that we added with the previous patches (last updated with D46236 I think):
https://clang.llvm.org/docs/ReleaseNotes.html#new-compiler-flags
https://clang.llvm.org/docs/UsersManual.html#controlling-code-generation
http://llvm.org/docs/ReleaseNotes.html#non-comprehensive-list-of-changes-in-this-release

Let me know if you see anything that can be improved.

The text in first and last links suggests that such an optimization is always being done.

Tue, Jun 19, 2:51 PM
spatel added a comment to D48085: [DAGCombiner] restrict (float)((int) f) --> ftrunc with no-signed-zeros.

Should this adjust the ReleaseNotes?

The docs provide extra warning/workaround, and this change doesn't affect that language IMO (there's less chance we're going to break code, but we don't have to disclose that).

For reference, here are links to the docs that we added with the previous patches (last updated with D46236 I think):
https://clang.llvm.org/docs/ReleaseNotes.html#new-compiler-flags
https://clang.llvm.org/docs/UsersManual.html#controlling-code-generation
http://llvm.org/docs/ReleaseNotes.html#non-comprehensive-list-of-changes-in-this-release

Let me know if you see anything that can be improved.

The text in first and last links suggests that such an optimization is always being done.
But now it will only be done in presence of no-signed-zeros-fp-math attribute.
Which is controlled by -ffast-math (or maybe some more fine-grained option, too?)

Tue, Jun 19, 1:05 PM
spatel updated the diff for D48085: [DAGCombiner] restrict (float)((int) f) --> ftrunc with no-signed-zeros.

Patch updated:
Added a 'TODO' comment about using a FABS-based sequence if we can't ignore -0.0.

Tue, Jun 19, 12:30 PM
spatel added a comment to D48085: [DAGCombiner] restrict (float)((int) f) --> ftrunc with no-signed-zeros.

Should this adjust the ReleaseNotes?

Tue, Jun 19, 12:28 PM
spatel committed rL335067: [IR] move shuffle mask queries from TTI to ShuffleVectorInst.
[IR] move shuffle mask queries from TTI to ShuffleVectorInst
Tue, Jun 19, 11:48 AM
spatel closed D48236: [IR] move shuffle mask queries from TTI to ShuffleVectorInst.
Tue, Jun 19, 11:48 AM
spatel added a comment to D48085: [DAGCombiner] restrict (float)((int) f) --> ftrunc with no-signed-zeros.

Ping.

Tue, Jun 19, 11:41 AM
spatel added inline comments to D48236: [IR] move shuffle mask queries from TTI to ShuffleVectorInst.
Tue, Jun 19, 11:29 AM
spatel updated the diff for D48236: [IR] move shuffle mask queries from TTI to ShuffleVectorInst.

Patch updated:

  1. Improved header comment examples (use 'undef' rather than -1).
  2. Reimplemented logic to maximize code reuse (make use of isSingleSourceMask() internally).
  3. Added 'TODO' notes about allowing size-changing shuffles.
Tue, Jun 19, 10:43 AM
spatel added inline comments to D48236: [IR] move shuffle mask queries from TTI to ShuffleVectorInst.
Tue, Jun 19, 10:39 AM
spatel added inline comments to D48223: Allow binop C1, (select cc, CF, CT) -> select folding.
Tue, Jun 19, 8:50 AM

Mon, Jun 18

spatel added a comment to D48000: [InstSimplify][NFC] Add tests for some missed optimization opportunities in simplifyUnsignedRangeCheck().

Do you have commit privileges?

Mon, Jun 18, 4:27 PM
spatel added inline comments to D48223: Allow binop C1, (select cc, CF, CT) -> select folding.
Mon, Jun 18, 4:04 PM
spatel accepted D47909: Utilize new SDNode flag functionality to expand current support for fadd.

LGTM - see inline for a nit.

Mon, Jun 18, 3:39 PM
spatel added a reviewer for D46380: [InstSimplify] Adding safety checks for 'shl', 'ashr' and 'lshr': lebedev.ri.

I see the motivation better now, but I'm still worried about using ValueTracking.

Mon, Jun 18, 3:11 PM
spatel added inline comments to D48223: Allow binop C1, (select cc, CF, CT) -> select folding.
Mon, Jun 18, 2:01 PM
spatel accepted D48289: refactor of visitFADD for AllowNewConst cases.

I'm not sure how to test that potential late constant creation problem (see rL334977), but this change looks safe and does what the comment says, so LGTM.

Mon, Jun 18, 1:13 PM
spatel committed rL334977: [x86] regenerate checks and adjust tests.
[x86] regenerate checks and adjust tests
Mon, Jun 18, 1:09 PM
spatel requested changes to D47983: [IR][PatternMatch] m_APInt(): allow undef elements..

On 2nd thought, I confused myself. :)
This patch is potentially dangerous - although it's hard to find cases where that danger propagates.
As with the earlier undef matcher changes, we have to audit the users to make sure they are not incorrectly using the matcher to propagate a value with undefs where it's not safe.

Mon, Jun 18, 11:29 AM
spatel accepted D47983: [IR][PatternMatch] m_APInt(): allow undef elements..

Ping.
Or is it intentional for m_APInt not to do this?

Mon, Jun 18, 10:07 AM
spatel added inline comments to D48229: [NFC][SCEV] Add tests related to bit masking (PR37793).
Mon, Jun 18, 9:20 AM
spatel added inline comments to D47909: Utilize new SDNode flag functionality to expand current support for fadd.
Mon, Jun 18, 8:51 AM
spatel accepted D48023: [SLPVectorizer] Tidyup isShuffle helper.

@spatel Should this wait for D48236 or can I commit it now?

Mon, Jun 18, 8:10 AM

Sun, Jun 17

spatel updated the diff for D48236: [IR] move shuffle mask queries from TTI to ShuffleVectorInst.

Uploaded the wrong draft - went overboard on the 'const &' on that last one. An ArrayRef is a const ref.

Sun, Jun 17, 4:27 PM
spatel updated the diff for D48236: [IR] move shuffle mask queries from TTI to ShuffleVectorInst.

Patch updated:
Redefine all of the Constant* functions in terms of ArrayRef<int> parent implementations. This makes the code slightly cleaner and might open up the possibility of removing some redundant code from the backend.

Sun, Jun 17, 4:17 PM
spatel updated the diff for D48236: [IR] move shuffle mask queries from TTI to ShuffleVectorInst.

Patch updated:

  1. Fixed undef checks to be "x == -1" rather than "x < 0".
  2. Added assertions for inputs and logic.
  3. Added more unit tests (the last group is based on the mask examples that I added to the header file code comments...would be extra embarrassing to get those wrong!).
  4. Reduced confusing comment for transpose (and restated the canonical TRN1 / TRN2 examples for clarity).
  5. Changed all 'unsigned' types to 'int' so we don't have to cast (this is always an annoyance with mask values).
  6. Changed variable names and added locals to try to make the logic clearer and more uniform.
Sun, Jun 17, 7:05 AM

Sat, Jun 16

spatel added inline comments to D48236: [IR] move shuffle mask queries from TTI to ShuffleVectorInst.
Sat, Jun 16, 1:33 PM
spatel added inline comments to D48223: Allow binop C1, (select cc, CF, CT) -> select folding.
Sat, Jun 16, 12:52 PM
spatel requested changes to D48223: Allow binop C1, (select cc, CF, CT) -> select folding.
Sat, Jun 16, 7:10 AM

Fri, Jun 15

spatel added inline comments to D47909: Utilize new SDNode flag functionality to expand current support for fadd.
Fri, Jun 15, 3:28 PM
spatel created D48236: [IR] move shuffle mask queries from TTI to ShuffleVectorInst.
Fri, Jun 15, 1:31 PM
spatel accepted D47954: Utilize new SDNode flag functionality to expand current support for fdiv.

LGTM.

Fri, Jun 15, 12:33 PM
spatel accepted D47922: [InstSimplify] Fix some missed optimization opportunities in simplifyUnsignedRangeCheck().

LGTM too. Thanks for finding the bug and working through the steps to make good patches.

Fri, Jun 15, 9:50 AM

Thu, Jun 14

spatel committed rL334759: [x86] be more selective about converting 'and' to shuffle (PR37749).
[x86] be more selective about converting 'and' to shuffle (PR37749)
Thu, Jun 14, 12:59 PM
spatel accepted D48057: easing the constraint for isNegatibleForFree and GetNegatedExpression.

LGTM.

Thu, Jun 14, 12:42 PM
spatel committed rL334744: [x86] add tests for AVX1 FP logic op abuse (PR37749); NFC.
[x86] add tests for AVX1 FP logic op abuse (PR37749); NFC
Thu, Jun 14, 11:12 AM
spatel accepted D48180: updating isNegatibleForFree and GetNegatedExpression with fmf for fadd.
Thu, Jun 14, 10:08 AM
spatel added a comment to D48180: updating isNegatibleForFree and GetNegatedExpression with fmf for fadd.

LGTM.

Thu, Jun 14, 10:08 AM
spatel added reviewers for D47918: Utilize new SDNode flag functionality to expand current support for fma: rampitec, nhaehnle, nemanjai.

Adding more potential FMA reviewers. There may be some tricky bits here in the interaction between the FMF, the global settings, and the TLI hooks. See for example D28675.

Thu, Jun 14, 8:03 AM
spatel added inline comments to D48057: easing the constraint for isNegatibleForFree and GetNegatedExpression.
Thu, Jun 14, 7:36 AM
spatel added inline comments to D47909: Utilize new SDNode flag functionality to expand current support for fadd.
Thu, Jun 14, 7:16 AM
spatel added inline comments to D45616: [X86] Lower _mm[256|512]_cmp[.]_mask intrinsics to native llvm IR.
Thu, Jun 14, 6:50 AM

Wed, Jun 13

spatel added a comment to D47963: [LangRef] Clarify that nnan and ninf don't produce undef or poison..

This reads clearer to me by avoiding the potential to confuse with UB. Should the same blurb apply to nsz with -0.0?

Wed, Jun 13, 3:25 PM
spatel updated subscribers of D48057: easing the constraint for isNegatibleForFree and GetNegatedExpression.
Wed, Jun 13, 2:42 PM
spatel accepted D47993: [x86] fix mappings of cvttp2si x86 intrinsics to x86-specific nodes and isel patterns (PR37551).

Might want to get a 2nd opinion on 512 though since I'm not as familiar with that.

Wed, Jun 13, 2:20 PM
spatel added a comment to D48134: [CodeGen] make nan builtins pure rather than const (PR37778).

Can we mark these as argmemonly?

Wed, Jun 13, 2:08 PM
spatel added inline comments to D48057: easing the constraint for isNegatibleForFree and GetNegatedExpression.
Wed, Jun 13, 12:00 PM
spatel added inline comments to D45616: [X86] Lower _mm[256|512]_cmp[.]_mask intrinsics to native llvm IR.
Wed, Jun 13, 11:15 AM