spatel (Sanjay Patel)
User

Projects

User does not belong to any projects.

User Details

User Since
May 22 2014, 1:24 PM (220 w, 6 d)

Recent Activity

Today

spatel added a comment to D50804: DAG: Fix isKnownNeverNaN for basic non-sNaN cases.

The change should be visible in tests because there's a DAGCombine that uses isKnownNeverNaN()?

Thu, Aug 16, 6:54 AM
spatel committed rL339875: [InstCombine] move vector compare before same-shuffled ops.
[InstCombine] move vector compare before same-shuffled ops
Thu, Aug 16, 5:53 AM
spatel added inline comments to D50827: [DAGCombiner] Don't reassociate operations that have the vector reduction flag set..
Thu, Aug 16, 5:08 AM

Yesterday

spatel added a reviewer for D50791: [SelectionDAG] unroll unsupported vector FP ops earlier to avoid libcalls on undef elements (PR38527): hfinkel.
Wed, Aug 15, 3:11 PM
spatel added a comment to D50791: [SelectionDAG] unroll unsupported vector FP ops earlier to avoid libcalls on undef elements (PR38527).

Thinking forward to a future where we have SVML library support for v4f32 and a v8f32 sin/cos. What should we do for v5f32?

Wed, Aug 15, 2:51 PM
spatel updated the diff for D50791: [SelectionDAG] unroll unsupported vector FP ops earlier to avoid libcalls on undef elements (PR38527).

Make the vector type check an assert instead of an actual condition for the transform (no test diffs).

Wed, Aug 15, 1:23 PM
spatel added inline comments to D50791: [SelectionDAG] unroll unsupported vector FP ops earlier to avoid libcalls on undef elements (PR38527).
Wed, Aug 15, 1:20 PM
spatel added inline comments to D50791: [SelectionDAG] unroll unsupported vector FP ops earlier to avoid libcalls on undef elements (PR38527).
Wed, Aug 15, 12:52 PM
spatel accepted D50036: [SLC] Expand the simplification of pow(x, 0.5) to sqrt(x).

LGTM

Wed, Aug 15, 12:22 PM
spatel created D50791: [SelectionDAG] unroll unsupported vector FP ops earlier to avoid libcalls on undef elements (PR38527).
Wed, Aug 15, 10:53 AM
spatel accepted D50775: [InstCombine] Fix IC trying to create a xor of pointer types.

LGTM

Wed, Aug 15, 10:42 AM
spatel committed rL339793: [AArch64] add tests for poor vector intrinsic lowering via legalization….
[AArch64] add tests for poor vector intrinsic lowering via legalization…
Wed, Aug 15, 10:07 AM
spatel committed rL339791: [x86] add fabs test for vector intrinsic to potential libcall bug; NFC.
[x86] add fabs test for vector intrinsic to potential libcall bug; NFC
Wed, Aug 15, 9:56 AM
spatel added a comment to D45653: [X86] Enable sibling-call optimization for functions returning structs.

@spatel Excuse me, can you explain, please, what's wrong with this patch now?

Ivan is my colleague, he have asked me to solve this review a week ago. And he have mentioned here, that he doesn't mind if someone will comandeer the review...

There was the very long delay with a reply, and I understand that it's not good (and I'm very sorry for that really). But now I want to solve this in the nearest future.

I really hope for your understanding.

Wed, Aug 15, 9:47 AM
spatel committed rL339790: [x86] add tests for poor vector intrinsic lowering via legalization (PR38527)….
[x86] add tests for poor vector intrinsic lowering via legalization (PR38527)…
Wed, Aug 15, 9:36 AM
spatel resigned from D45653: [X86] Enable sibling-call optimization for functions returning structs.
Wed, Aug 15, 8:38 AM
spatel added reviewers for D50775: [InstCombine] Fix IC trying to create a xor of pointer types: efriedma, lebedev.ri.

I've fixed this by doing a ptrtoint convert on the assumption that ptrtoints are free.

Wed, Aug 15, 8:32 AM

Tue, Aug 14

spatel added inline comments to D50036: [SLC] Expand the simplification of pow(x, 0.5) to sqrt(x).
Tue, Aug 14, 12:17 PM
spatel committed rL339713: [InstCombine] fix typos in tests; NFC.
[InstCombine] fix typos in tests; NFC
Tue, Aug 14, 12:14 PM
spatel committed rL339711: [InstCombine] add tests for pow->sqrt; NFC.
[InstCombine] add tests for pow->sqrt; NFC
Tue, Aug 14, 12:06 PM
spatel added a reviewer for D50714: [InstCombine] Fold Select with binary op - FP opcodes: lebedev.ri.
Tue, Aug 14, 11:13 AM
spatel added inline comments to D50714: [InstCombine] Fold Select with binary op - FP opcodes.
Tue, Aug 14, 10:54 AM
spatel updated subscribers of D50036: [SLC] Expand the simplification of pow(x, 0.5) to sqrt(x).
Tue, Aug 14, 9:11 AM
spatel added inline comments to D49273: [SLC] Expand the simplification of pow({e,2}, y) to exp{,2}(y).
Tue, Aug 14, 8:27 AM
spatel committed rL339683: [InstCombine] regenerate checks; NFC.
[InstCombine] regenerate checks; NFC
Tue, Aug 14, 8:21 AM
spatel committed rL339681: [InstCombine] regenerate checks; NFC.
[InstCombine] regenerate checks; NFC
Tue, Aug 14, 8:19 AM
spatel added inline comments to D50036: [SLC] Expand the simplification of pow(x, 0.5) to sqrt(x).
Tue, Aug 14, 6:27 AM

Mon, Aug 13

spatel committed rL339618: [SimplifyLibCalls] don't drop fast-math-flags on trig reflection folds (retry….
[SimplifyLibCalls] don't drop fast-math-flags on trig reflection folds (retry…
Mon, Aug 13, 2:50 PM
spatel committed rL339609: revert r339608 - [SimplifyLibCalls] don't drop fast-math-flags on trig….
revert r339608 - [SimplifyLibCalls] don't drop fast-math-flags on trig…
Mon, Aug 13, 1:21 PM
spatel accepted D50465: [InstCombine] Optimize redundant 'signed truncation check pattern'..

Forgot to click 'Accept' here in Phab.

Mon, Aug 13, 1:16 PM
spatel committed rL339608: [SimplifyLibCalls] don't drop fast-math-flags on trig reflection folds.
[SimplifyLibCalls] don't drop fast-math-flags on trig reflection folds
Mon, Aug 13, 1:15 PM
spatel added a comment to D50465: [InstCombine] Optimize redundant 'signed truncation check pattern'..

About the autogen script problem - we could hack that to use something less likely than 'TMP' as its regex string (eg, 'AUTOGEN'), but that will result in significant test file churn if someone updates an existing file.

Mon, Aug 13, 12:54 PM
spatel committed rL339604: [SimplifyLibCalls] add reflection fold for -sin(-x) (PR38458).
[SimplifyLibCalls] add reflection fold for -sin(-x) (PR38458)
Mon, Aug 13, 12:25 PM
spatel committed rL339598: [InstCombine] add more tests for trig reflections; NFC (PR38458).
[InstCombine] add more tests for trig reflections; NFC (PR38458)
Mon, Aug 13, 11:35 AM
spatel committed rL339588: [SimplifyLibCalls] reduce code for optimizeCos; NFCI.
[SimplifyLibCalls] reduce code for optimizeCos; NFCI
Mon, Aug 13, 10:41 AM
spatel committed rL339579: [InstCombine] auto-generate full checks and add cos intrinsic test; NFC.
[InstCombine] auto-generate full checks and add cos intrinsic test; NFC
Mon, Aug 13, 9:29 AM
spatel accepted D50464: [InstCombine][NFC] Tests for 'signed truncation check' optimization.

Tests LGTM. It might help me or other reviewers see how these patterns escape the current set of transforms in foldAndOfICmps(), so please commit.

Mon, Aug 13, 8:20 AM
spatel added inline comments to D50465: [InstCombine] Optimize redundant 'signed truncation check pattern'..
Mon, Aug 13, 8:08 AM

Sun, Aug 12

spatel added inline comments to D50582: ValueTracking: Handle more instructions in isKnownNeverNaN.
Sun, Aug 12, 2:47 PM
spatel committed rL339519: [InstCombine] fix/enhance fadd/fsub factorization.
[InstCombine] fix/enhance fadd/fsub factorization
Sun, Aug 12, 8:49 AM
spatel accepted D50190: [InstCombine] Fold Select with binary op - non-commutative opcodes.

LGTM

Sun, Aug 12, 8:07 AM
spatel committed rL339518: [InstCombine] move/add tests for fadd/fsub factorization; NFC.
[InstCombine] move/add tests for fadd/fsub factorization; NFC
Sun, Aug 12, 8:06 AM

Fri, Aug 10

spatel added inline comments to D50035: [SLC] Expand simplification of pow() for vector types.
Fri, Aug 10, 2:13 PM
spatel committed rL339470: [InstCombine] add tests for fsub factorization; NFC.
[InstCombine] add tests for fsub factorization; NFC
Fri, Aug 10, 2:01 PM
spatel added a comment to D50190: [InstCombine] Fold Select with binary op - non-commutative opcodes.
  • Rebased/updated
Fri, Aug 10, 1:35 PM
spatel committed rL339469: [InstCombine] rearrange code for foldSelectBinOpIdentity; NFCI.
[InstCombine] rearrange code for foldSelectBinOpIdentity; NFCI
Fri, Aug 10, 1:31 PM
spatel accepted D50035: [SLC] Expand simplification of pow() for vector types.

I suppose the extra check doesn't hurt anything, and this is all disorganized anyway. The simplifications of calls to constants should really be in InstSimplify.
The vector changes look correct, so LGTM.

Fri, Aug 10, 1:17 PM
spatel added inline comments to D50035: [SLC] Expand simplification of pow() for vector types.
Fri, Aug 10, 1:14 PM
spatel committed rL339467: [InstCombine] add tests to show disabling of libcall/intrinsic shrinking; NFC.
[InstCombine] add tests to show disabling of libcall/intrinsic shrinking; NFC
Fri, Aug 10, 1:13 PM
spatel committed rL339463: revert r339450 - [MS Demangler] Add conversion operator tests.
revert r339450 - [MS Demangler] Add conversion operator tests
Fri, Aug 10, 12:20 PM
spatel committed rL339453: [InstCombine] add/update tests for selectBinOpIdentity; NFC.
[InstCombine] add/update tests for selectBinOpIdentity; NFC
Fri, Aug 10, 10:21 AM
spatel committed rL339446: [InstCombine] revert r339439 - rearrange code for foldSelectBinOpIdentity.
[InstCombine] revert r339439 - rearrange code for foldSelectBinOpIdentity
Fri, Aug 10, 9:13 AM
spatel added inline comments to D50035: [SLC] Expand simplification of pow() for vector types.
Fri, Aug 10, 9:03 AM
spatel added a comment to D50190: [InstCombine] Fold Select with binary op - non-commutative opcodes.

You should be able to adjust 2 lines of code to allow non-commutative ops after:
rL339439

Fri, Aug 10, 8:22 AM
spatel committed rL339439: [InstCombine] rearrange code for foldSelectBinOpIdentity; NFCI.
[InstCombine] rearrange code for foldSelectBinOpIdentity; NFCI
Fri, Aug 10, 8:12 AM
spatel added a comment to D50190: [InstCombine] Fold Select with binary op - non-commutative opcodes.
  • Added/fixed/updated tests (ok now?)
Fri, Aug 10, 6:39 AM

Thu, Aug 9

spatel added inline comments to D50035: [SLC] Expand simplification of pow() for vector types.
Thu, Aug 9, 3:39 PM
spatel committed rL339396: [InstSimplify] move minnum/maxnum with Inf folds from instcombine.
[InstSimplify] move minnum/maxnum with Inf folds from instcombine
Thu, Aug 9, 3:21 PM
spatel accepted D50338: ValueTracking: Start enhancing isKnownNeverNaN.

LGTM

Thu, Aug 9, 1:17 PM
spatel added inline comments to D50035: [SLC] Expand simplification of pow() for vector types.
Thu, Aug 9, 1:10 PM
spatel committed rL339368: [InstCombine] allow fsub+fmul FMF folds for vectors.
[InstCombine] allow fsub+fmul FMF folds for vectors
Thu, Aug 9, 11:43 AM
spatel committed rL339361: [InstCombine] add vector tests for fsub+fmul; NFC.
[InstCombine] add vector tests for fsub+fmul; NFC
Thu, Aug 9, 10:41 AM
spatel committed rL339359: [SelectionDAG] try harder to convert funnel shift to rotate.
[SelectionDAG] try harder to convert funnel shift to rotate
Thu, Aug 9, 10:27 AM
spatel closed D50091: [SelectionDAG] try harder to convert funnel shift to rotate.
Thu, Aug 9, 10:26 AM
spatel committed rL339349: [InstCombine] reduce code duplication; NFC.
[InstCombine] reduce code duplication; NFC
Thu, Aug 9, 8:07 AM
spatel added a comment to D50091: [SelectionDAG] try harder to convert funnel shift to rotate.

Ping.

Thu, Aug 9, 7:15 AM
spatel added inline comments to D50465: [InstCombine] Optimize redundant 'signed truncation check pattern'..
Thu, Aug 9, 7:12 AM
spatel updated subscribers of D50499: [X86] Constant folding of adds/subs intrinsics.

I'm not finding the mails where the extraction of target-specific code came up, but my vague memory says @rnk had some ideas about how to improve things.

Thu, Aug 9, 5:34 AM
spatel added reviewers for D50499: [X86] Constant folding of adds/subs intrinsics: chandlerc, efriedma.

The title of this patch includes "Constant folding" - shouldn't the code be in ConstantFolding.cpp or some descendant of that?

Thu, Aug 9, 5:18 AM
spatel accepted D50195: extend folding fsub/fadd to fneg for FMF.

LGTM

Thu, Aug 9, 4:33 AM

Wed, Aug 8

spatel added a comment to D50195: extend folding fsub/fadd to fneg for FMF.

At this point there isn't any code either way that checks for fine...

Wed, Aug 8, 7:20 PM
spatel added inline comments to D50195: extend folding fsub/fadd to fneg for FMF.
Wed, Aug 8, 7:14 PM
spatel added a comment to D50195: extend folding fsub/fadd to fneg for FMF.

Even with all the above changes, the opportunity persists. We need to move:

// fold (fsub A, (fneg B)) -> (fadd A, B)

if (isNegatibleForFree(N1, LegalOperations, TLI, &Options))
  return DAG.getNode(ISD::FADD, DL, VT, N0,
                     GetNegatedExpression(N1, DAG, LegalOperations), Flags);

after the match as it sweeps up part of the key expression, and never completes the rest, leaving it partially optimized for some cases (in the test case noted, its for zero gain). Basically, this key context should be after all the major specialization matches to give each of the others a chance to complete.

Wed, Aug 8, 4:16 PM
spatel committed rL339300: [x86] add test for commuted variant for fsub fold; NFC.
[x86] add test for commuted variant for fsub fold; NFC
Wed, Aug 8, 4:07 PM
spatel committed rL339299: [DAGCombiner] loosen constraints for fsub+fadd fold.
[DAGCombiner] loosen constraints for fsub+fadd fold
Wed, Aug 8, 4:05 PM
spatel committed rL339298: [DAGCombiner] move fadd simplification ahead of other folds.
[DAGCombiner] move fadd simplification ahead of other folds
Wed, Aug 8, 3:47 PM
spatel committed rL339293: [x86] add tests for fsub+fadd with FMF; NFC.
[x86] add tests for fsub+fadd with FMF; NFC
Wed, Aug 8, 3:18 PM
spatel added a comment to D50195: extend folding fsub/fadd to fneg for FMF.

I'm still confused. I don't see why we need to check isNegatibleForFree().

Wed, Aug 8, 9:38 AM
spatel committed rL339267: [InstCombine] fold fadd+fsub with common operand.
[InstCombine] fold fadd+fsub with common operand
Wed, Aug 8, 9:19 AM
spatel committed rL339266: [InstCombine] fold fsub+fsub with common operand.
[InstCombine] fold fsub+fsub with common operand
Wed, Aug 8, 9:05 AM
spatel committed rL339263: [InstCombine] add tests for fsub folds; NFC.
[InstCombine] add tests for fsub folds; NFC
Wed, Aug 8, 8:45 AM
spatel committed rL339248: [InstCombine] fold fneg into constant operand of fmul/fdiv.
[InstCombine] fold fneg into constant operand of fmul/fdiv
Wed, Aug 8, 7:29 AM
spatel closed D50417: [InstCombine] fold fneg into constant operand of fmul/fdiv.
Wed, Aug 8, 7:29 AM
spatel accepted D50417: [InstCombine] fold fneg into constant operand of fmul/fdiv.

Marking as accepted here on Phab to avoid unknown state when committing.

Wed, Aug 8, 7:28 AM
spatel added a comment to D50168: [Builtins] Implement __builtin_clrsb to be compatible with gcc.

About the bit hacking: I don't think clang should be in the optimization business. We should be able to take the most obvious/simple representation for this builtin and reduce it as needed (either in instcombine or the backend). So it would be better to use the version with the subtract rather than shift/or. That version corresponds more directly to the code in APInt::getMinSignedBits()?

Wed, Aug 8, 6:21 AM
spatel accepted D50301: [InstCombine] De Morgan: sink 'not' into 'xor' (PR38446).

LGTM

Wed, Aug 8, 5:41 AM
spatel added a comment to D50081: [ValueTracking] fix maxnum miscompile for cannotBeOrderedLessThanZero (PR37776).

Maybe this fix be should merged into 7.0 too?

@hans

Sounds good to me, but it needs to be committed first obviously :-)

Merged in r339234.

Wed, Aug 8, 5:32 AM

Tue, Aug 7

spatel created D50417: [InstCombine] fold fneg into constant operand of fmul/fdiv.
Tue, Aug 7, 4:29 PM
spatel committed rL339203: [InstCombine] add tests for fneg fold including FMF; NFC.
[InstCombine] add tests for fneg fold including FMF; NFC
Tue, Aug 7, 4:25 PM
spatel committed rL339200: [InstCombine] fix FP constant in test; NFC.
[InstCombine] fix FP constant in test; NFC
Tue, Aug 7, 4:04 PM
spatel committed rL339195: [InstCombine] add tests for fneg of fmul/fdiv with constant; NFC.
[InstCombine] add tests for fneg of fmul/fdiv with constant; NFC
Tue, Aug 7, 3:31 PM
spatel added a comment to D49040: [SLC] Simplify pow(x, 0.333...) to cbrt(x).

As we discussed in D49306, I agree that this is probably a good perf transform, but I don't think we've shown any compelling reason to do this in IR vs. DAGCombiner.

Tue, Aug 7, 1:46 PM
spatel committed rL339176: [InstSimplify] fold fsub+fadd with common operand.
[InstSimplify] fold fsub+fadd with common operand
Tue, Aug 7, 1:33 PM
spatel committed rL339174: [InstSimplify] fold fadd+fsub with common operand.
[InstSimplify] fold fadd+fsub with common operand
Tue, Aug 7, 1:24 PM
spatel committed rL339171: [InstSimplify] fold fsub+fsub with common operand.
[InstSimplify] fold fsub+fsub with common operand
Tue, Aug 7, 1:15 PM
spatel committed rL339168: [InstSimplify] add tests for fadd/fsub; NFC.
[InstSimplify] add tests for fadd/fsub; NFC
Tue, Aug 7, 12:49 PM
spatel added a comment to D50195: extend folding fsub/fadd to fneg for FMF.

I like these test versions a little better, they show an opportunity yet to be had for the IR optimizer.

Tue, Aug 7, 10:42 AM
spatel added inline comments to D50301: [InstCombine] De Morgan: sink 'not' into 'xor' (PR38446).
Tue, Aug 7, 10:26 AM
spatel added a comment to D50301: [InstCombine] De Morgan: sink 'not' into 'xor' (PR38446).

I may be starting a substantial yak shaving with the inline question, but I wonder if it wouldn't be better to canonicalize with the 'not' after the 'xor' (ie, the inverse of this patch). I don't have a motivating example yet, but that just seems more natural rather than semi-arbitralily choosing an operand to invert.

Tue, Aug 7, 10:04 AM
spatel added a reviewer for D50190: [InstCombine] Fold Select with binary op - non-commutative opcodes: lebedev.ri.

Let's handle this one step at a time. Please remove the FP changes. Let's make this patch only about handling the non-commutative integer binops.

Tue, Aug 7, 9:31 AM