sanjoy (Sanjoy Das)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 6 2014, 4:30 PM (175 w, 2 d)

Recent Activity

Yesterday

sanjoy added a comment to D38928: [LoopInfo][Refactor] Make SetLoopAlreadyUnrolled a member function of the Loop Pass, NFC..

LGTM

I think the Loop class has grown quite a bit, and we should pull out some of the utility functions into a separate LoopUtils namespace, but this patch does not need to be blocked on that.

Just out of curiosity, what are the principle to tell whether a function should be a member function of the Loop class or a utilities?

Sun, Oct 15, 12:00 AM

Sat, Oct 14

sanjoy accepted D38928: [LoopInfo][Refactor] Make SetLoopAlreadyUnrolled a member function of the Loop Pass, NFC..

I think the Loop class has grown quite a bit, and we should pull out some of the utility functions into a separate LoopUtils namespace, but this patch does not need to be blocked on that.

Sat, Oct 14, 8:27 PM
sanjoy accepted D38924: Fix `FaultMaps` crash when the out streamer is reused.
Sat, Oct 14, 5:15 PM
sanjoy edited reviewers for D38924: Fix `FaultMaps` crash when the out streamer is reused, added: skatkov; removed: sanjoy.
Sat, Oct 14, 5:15 PM
sanjoy edited reviewers for D38925: Fix implicit null check with negative offset, added: skatkov; removed: sanjoy.
Sat, Oct 14, 5:12 PM
sanjoy added inline comments to D38924: Fix `FaultMaps` crash when the out streamer is reused.
Sat, Oct 14, 3:54 PM

Fri, Oct 13

sanjoy committed rL315713: [SCEV] Maintain and use a loop->loop invalidation dependency.
[SCEV] Maintain and use a loop->loop invalidation dependency
Fri, Oct 13, 10:14 AM
sanjoy closed D38435: [SCEV] Maintain and use a loop->loop invalidation dependency by committing rL315713: [SCEV] Maintain and use a loop->loop invalidation dependency.
Fri, Oct 13, 10:14 AM
sanjoy added inline comments to D38825: [SCEV] Teach SCEV to find maxBECount when loop endbound is variant.
Fri, Oct 13, 9:59 AM

Thu, Oct 12

sanjoy updated the diff for D38435: [SCEV] Maintain and use a loop->loop invalidation dependency.
  • address review
Thu, Oct 12, 11:14 PM
sanjoy committed rL315672: [SCEV] Maintain loop use lists, and use them in forgetLoop.
[SCEV] Maintain loop use lists, and use them in forgetLoop
Thu, Oct 12, 10:51 PM
sanjoy closed D38434: [SCEV] Maintain loop use lists, and use them in forgetLoop by committing rL315672: [SCEV] Maintain loop use lists, and use them in forgetLoop.
Thu, Oct 12, 10:51 PM
sanjoy requested changes to D37660: [ScalarEvolution] Handling for ICmp occuring in the evolution chain..
Thu, Oct 12, 10:41 PM
sanjoy planned changes to D38433: Introduce a specialized data structure to be used in a subsequent change.

(Haven't addressed the code comments yet since the design isn't settled)

Have you considered building a ChunkedVector instead of a ChunkedList? Specifically, there is a great trick where you use a single index with the low bits being an index into the chunk and the high bits being an index into a vector of pointers. It has many of the benefits you list and is a bit simpler I think. It also supports essentially the entire vector API if desired. Both bi-directional and even random access are reasonably efficient. Good locality, etc.

With a vector-of-buffers implementation, I'm a bit worried about the space overhead on the smaller cases. For instance, this is the histogram of how this data structure is populated from a clang-bootstrap (also in https://reviews.llvm.org/D38434):

     Count: 731310
       Min: 1
      Mean: 8.555150
50th %tile: 4
95th %tile: 25
99th %tile: 53
       Max: 433

If I used a vector-of-buffers, I will either have to recompute the capacity and end (of the last buffer) on every insert (which will require an additional deref and some computation) or have to keep two words in the data structure over the three that smallvector keeps anyway. This adds a lot of relative overhead on the median case (4 elements). In fact, the current situation of two extra words also qualifies as a "lot" of relative overhead IMO, and I want to think of an SSO to improve the situation.

Hold on, the objects here are just pointers? Then none of this really makes sense to me...

Chunked data structures seem to make the most sense if moving the objects is really expensive and/or the objects are really large.

For pointers, why not just a vector?

Thu, Oct 12, 10:29 PM
sanjoy added inline comments to D38856: [IPSCCP] Remove calls without side effects.
Thu, Oct 12, 1:00 PM

Wed, Oct 11

sanjoy added a comment to D37660: [ScalarEvolution] Handling for ICmp occuring in the evolution chain..

I went through your example, and I think I know what you need to do -- while you cannot assume that a condition feeding into the loop latch is true generally, you can assume it is true *in the context* of a add recurrence increment. That is, in createAddRecFromPHI, under // Create an add with everything but the specified operand., if an operand to the addition is a loop variant value that is the loop latch condition then you can assume said loop variant value to be the constant 1.

Wed, Oct 11, 5:26 PM
sanjoy added a comment to D37660: [ScalarEvolution] Handling for ICmp occuring in the evolution chain..

I don't understand what you mean D:
does it meaning do I need to upload raw source code?

Wed, Oct 11, 5:02 PM
sanjoy accepted D38814: [SCEV] Properly handle the case of a non-constant start with a zero accum in ScalarEvolution::createAddRecFromPHIWithCastsImpl.

LGTM

Wed, Oct 11, 11:45 AM

Tue, Oct 10

sanjoy added a comment to D38433: Introduce a specialized data structure to be used in a subsequent change.

(Haven't addressed the code comments yet since the design isn't settled)

Tue, Oct 10, 12:26 PM
sanjoy updated the summary of D38434: [SCEV] Maintain loop use lists, and use them in forgetLoop.
Tue, Oct 10, 11:52 AM

Mon, Oct 9

sanjoy added a comment to D38433: Introduce a specialized data structure to be used in a subsequent change.

Looks a fair bit like std::deque - what're the tradeoffs between the two?

Mon, Oct 9, 5:10 PM

Wed, Oct 4

sanjoy committed rL314938: Do not call Loop::getName on possibly dead loops.
Do not call Loop::getName on possibly dead loops
Wed, Oct 4, 3:04 PM
sanjoy added a comment to D38494: [ScalarEvolution] Handling for ICmp occuring in the evolution chain..

With this IR:

Wed, Oct 4, 12:11 PM

Tue, Oct 3

sanjoy added a comment to D38494: [ScalarEvolution] Handling for ICmp occuring in the evolution chain..
int values[11];
int i = 0;
do {

values[i] = i != 10;
} while (i++ != 10);

will be removed. from LoopDeletion.

int values[11];
int i = 0;
do {

values[i] = i != 10;
} while (i++ != 10);


print(values[10]);

will not be even optimized.
(this is what I thought)

Tue, Oct 3, 9:23 PM
sanjoy added a comment to D38494: [ScalarEvolution] Handling for ICmp occuring in the evolution chain..

can you explain me why? (I may I didn't understood why do we need to process without loop latch)
I thought without latch cannot be evoluted from ICmp evolution.

without latch loop have to me evoluted from previous SCEV operation. isn't it?

Tue, Oct 3, 7:28 PM
sanjoy requested changes to D38494: [ScalarEvolution] Handling for ICmp occuring in the evolution chain..
Tue, Oct 3, 10:11 AM

Fri, Sep 29

sanjoy created D38435: [SCEV] Maintain and use a loop->loop invalidation dependency.
Fri, Sep 29, 3:20 PM
sanjoy created D38434: [SCEV] Maintain loop use lists, and use them in forgetLoop.
Fri, Sep 29, 3:20 PM
sanjoy created D38433: Introduce a specialized data structure to be used in a subsequent change.
Fri, Sep 29, 3:19 PM
sanjoy committed rL314539: Use LLVM_ENABLE_ABI_BREAKING_CHECKS correctly.
Use LLVM_ENABLE_ABI_BREAKING_CHECKS correctly
Fri, Sep 29, 10:19 AM

Thu, Sep 28

sanjoy committed rL314482: Revert "[BypassSlowDivision] Improve our handling of divisions by constants".
Revert "[BypassSlowDivision] Improve our handling of divisions by constants"
Thu, Sep 28, 5:56 PM

Wed, Sep 27

sanjoy committed rL314375: Use a BumpPtrAllocator for Loop objects.
Use a BumpPtrAllocator for Loop objects
Wed, Sep 27, 7:47 PM
sanjoy closed D38201: Use a BumpPtrAllocator for Loop objects by committing rL314375: Use a BumpPtrAllocator for Loop objects.
Wed, Sep 27, 7:47 PM
sanjoy updated the diff for D38201: Use a BumpPtrAllocator for Loop objects.

Remove the last remaining use of isInvalid() (I had missed this one but it
showed up when building in release mode).

Wed, Sep 27, 4:54 PM
sanjoy updated the diff for D38201: Use a BumpPtrAllocator for Loop objects.

With this update, we no longer have the UB in the legacy PM

Wed, Sep 27, 3:08 PM
sanjoy committed rL314352: Return the LoopUnrollResult from tryToUnrollLoop; NFC.
Return the LoopUnrollResult from tryToUnrollLoop; NFC
Wed, Sep 27, 2:47 PM
sanjoy committed rL314351: LoopDeletion: use return value instead of passing in LPMUpdater; NFC.
LoopDeletion: use return value instead of passing in LPMUpdater; NFC
Wed, Sep 27, 2:47 PM
sanjoy committed rL314350: Rename LoopUnrollStatus to LoopUnrollResult; NFC.
Rename LoopUnrollStatus to LoopUnrollResult; NFC
Wed, Sep 27, 2:46 PM

Tue, Sep 26

sanjoy added a comment to D38201: Use a BumpPtrAllocator for Loop objects.

There is a problem with this patch -- the legacy pass manager checks if a loop is valid or not by querying isInvalid, which is incorrect; since we call the destructor for loop objects to mark them as "invalid" calling isInvalid for possibly invalid loop objects has UB. We already have this UB in User::operator delete though, and so given that and that the legacy pass manager is going away at some point, perhaps this is okay? Alternatively, I could add a mechanism on the legacy loop pass manager that lets passes denote loop objects to have been deleted (by remembering the pointers in a set) and use that to decide if a Loop object is valid or not (and with that, I can guard isInvalid with #ifndef NDEBUG).

Sadly, I think this would be problematic. I think it'll trip ASan's use-after-scope or use-after-lifetime checks.... =/ But maybe there is a way around that. Or maybe just implement the solution you suggest. Dunno, but definitely worth checking.

Tue, Sep 26, 6:15 PM
sanjoy requested changes to D35741: Add MemorySSA alternative to AliasSetTracker in LICM..

I have added some generic style comments.

Tue, Sep 26, 5:42 PM
sanjoy committed rL314253: [BypassSlowDivision] Improve our handling of divisions by constants.
[BypassSlowDivision] Improve our handling of divisions by constants
Tue, Sep 26, 3:35 PM
sanjoy closed D38265: [BypassSlowDivision] Improve our handling of divisions by constants by committing rL314253: [BypassSlowDivision] Improve our handling of divisions by constants.
Tue, Sep 26, 3:35 PM
sanjoy updated the diff for D38265: [BypassSlowDivision] Improve our handling of divisions by constants.
  • update comments
Tue, Sep 26, 3:30 PM
sanjoy added inline comments to D38265: [BypassSlowDivision] Improve our handling of divisions by constants.
Tue, Sep 26, 3:30 PM
sanjoy accepted D38272: [SimplifyIndVar] Constant fold IV users.

Nice!

Tue, Sep 26, 3:29 PM
sanjoy created D38265: [BypassSlowDivision] Improve our handling of divisions by constants.
Tue, Sep 26, 3:29 PM

Mon, Sep 25

sanjoy added a comment to D38201: Use a BumpPtrAllocator for Loop objects.

There is a problem with this patch -- the legacy pass manager checks if a loop is valid or not by querying isInvalid, which is incorrect; since we call the destructor for loop objects to mark them as "invalid" calling isInvalid for possibly invalid loop objects has UB. We already have this UB in User::operator delete though, and so given that and that the legacy pass manager is going away at some point, perhaps this is okay? Alternatively, I could add a mechanism on the legacy loop pass manager that lets passes denote loop objects to have been deleted (by remembering the pointers in a set) and use that to decide if a Loop object is valid or not (and with that, I can guard isInvalid with #ifndef NDEBUG).

Mon, Sep 25, 11:49 AM
sanjoy accepted D37591: [LVI] Move LVILatticeVal class to separate header file (NFC)..

LGTM with nits

Mon, Sep 25, 5:03 AM
sanjoy accepted D38072: [SimplifyIndvar] Replace the srem used by IV if we can prove both of its operands are non-negative.

LGTM with comments addressed.

Mon, Sep 25, 5:02 AM
sanjoy added a comment to D33332: Possible typo in ProgrammersManual documentation.

The novice reader would associate shape with only two of the DerivedTypes: VectorType and ArrayType. How could a FunctionType or an IntegerType have a shape ?

I'm not sure that this is a typo -- the existing doc is probably talking about the shapes of the struct and array types.

Also, I didn't understand what was meant by a struct having a shape.

I raised this patch because I had misinterpreted it the first time I read it.

Mon, Sep 25, 5:02 AM
sanjoy updated the diff for D38201: Use a BumpPtrAllocator for Loop objects.
  • Update: Use Deallocate
Mon, Sep 25, 5:02 AM
sanjoy planned changes to D38201: Use a BumpPtrAllocator for Loop objects.
Mon, Sep 25, 5:02 AM
sanjoy created D38201: Use a BumpPtrAllocator for Loop objects.
Mon, Sep 25, 5:02 AM

Thu, Sep 21

sanjoy accepted D37888: [SCEV] Generalize folding of trunc(x)+n*trunc(y) into folding m*trunc(x)+n*trunc(y).

LGTM!

Thu, Sep 21, 11:00 PM
sanjoy committed rL313951: Rename markAsErased to erase, as pointed out in a previous review; NFC.
Rename markAsErased to erase, as pointed out in a previous review; NFC
Thu, Sep 21, 6:49 PM

Wed, Sep 20

sanjoy added inline comments to D37888: [SCEV] Generalize folding of trunc(x)+n*trunc(y) into folding m*trunc(x)+n*trunc(y).
Wed, Sep 20, 4:41 PM
sanjoy added inline comments to D38072: [SimplifyIndvar] Replace the srem used by IV if we can prove both of its operands are non-negative.
Wed, Sep 20, 10:59 AM
sanjoy added inline comments to D37888: [SCEV] Generalize folding of trunc(x)+n*trunc(y) into folding m*trunc(x)+n*trunc(y).
Wed, Sep 20, 10:29 AM
sanjoy requested changes to D38072: [SimplifyIndvar] Replace the srem used by IV if we can prove both of its operands are non-negative.

This is looking close; only some nitpicky comments inline

Wed, Sep 20, 10:16 AM
sanjoy added inline comments to D38055: Tighten the invariants around LoopBase::invalidate.
Wed, Sep 20, 10:01 AM

Tue, Sep 19

sanjoy requested changes to D38072: [SimplifyIndvar] Replace the srem used by IV if we can prove both of its operands are non-negative.
Tue, Sep 19, 8:56 PM
sanjoy committed rL313708: Tighten the invariants around LoopBase::invalidate.
Tighten the invariants around LoopBase::invalidate
Tue, Sep 19, 7:34 PM
sanjoy closed D38055: Tighten the invariants around LoopBase::invalidate by committing rL313708: Tighten the invariants around LoopBase::invalidate.
Tue, Sep 19, 7:33 PM
sanjoy added inline comments to D38055: Tighten the invariants around LoopBase::invalidate.
Tue, Sep 19, 7:21 PM
sanjoy added inline comments to D38055: Tighten the invariants around LoopBase::invalidate.
Tue, Sep 19, 6:27 PM
sanjoy updated the diff for D38055: Tighten the invariants around LoopBase::invalidate.
  • Address review comments
Tue, Sep 19, 6:27 PM
sanjoy committed rL313705: Clang-format few files to make later diffs leaner; NFC.
Clang-format few files to make later diffs leaner; NFC
Tue, Sep 19, 6:13 PM
sanjoy added inline comments to D37996: [LoopInfo] Make LoopBase and Loop destructors non-public.
Tue, Sep 19, 4:22 PM
sanjoy committed rL313695: [LoopInfo] Make LoopBase and Loop destructors non-public.
[LoopInfo] Make LoopBase and Loop destructors non-public
Tue, Sep 19, 4:20 PM
sanjoy closed D37996: [LoopInfo] Make LoopBase and Loop destructors non-public by committing rL313695: [LoopInfo] Make LoopBase and Loop destructors non-public.
Tue, Sep 19, 4:20 PM
sanjoy added a dependent revision for D37996: [LoopInfo] Make LoopBase and Loop destructors non-public: D38055: Tighten the invariants around LoopBase::invalidate.
Tue, Sep 19, 2:27 PM
sanjoy added a dependency for D38055: Tighten the invariants around LoopBase::invalidate: D37996: [LoopInfo] Make LoopBase and Loop destructors non-public.
Tue, Sep 19, 2:27 PM
sanjoy created D38055: Tighten the invariants around LoopBase::invalidate.
Tue, Sep 19, 2:26 PM

Mon, Sep 18

sanjoy created D37996: [LoopInfo] Make LoopBase and Loop destructors non-public.
Mon, Sep 18, 2:13 PM

Sep 15 2017

sanjoy requested changes to D37888: [SCEV] Generalize folding of trunc(x)+n*trunc(y) into folding m*trunc(x)+n*trunc(y).
Sep 15 2017, 4:35 PM

Sep 14 2017

sanjoy accepted D37870: Refactor collectChildrenInLoop to LoopUtils [NFC].

I added a couple of stylistic issues that were present in the earlier function. I'll leave it up to you if you want to fix these.

Sep 14 2017, 2:49 PM

Sep 13 2017

sanjoy committed rL313170: Fix a misleading phrase in the LangRef.
Fix a misleading phrase in the LangRef
Sep 13 2017, 11:50 AM
sanjoy closed D37432: Fix a misleading phrase in the LangRef by committing rL313170: Fix a misleading phrase in the LangRef.
Sep 13 2017, 11:50 AM
sanjoy added a comment to D37579: [InstCombine] Fix PR21780 Expansion of 256 bit vector loads fails to fold into shuffles.

Why not leave the original load in place, and tag the load metadata like !speculation_marker ? In either case, this needs a LangRef entry.

Sep 13 2017, 12:41 AM

Sep 12 2017

sanjoy accepted D37569: Rework loop predication pass.

I did not check the actual code changes carefully -- I've assumed they're in sync with the comments -- but let me know if you want me to take a look. The comments and the math bits LGTM.

Sep 12 2017, 11:35 PM
sanjoy added a comment to D37579: [InstCombine] Fix PR21780 Expansion of 256 bit vector loads fails to fold into shuffles.

Why not leave the original load in place, and tag the load metadata like !speculation_marker ? In either case, this needs a LangRef entry.

Sep 12 2017, 2:28 PM

Sep 11 2017

sanjoy accepted D35439: Make promoteLoopAccessesToScalars independent of AliasSet [NFC].
Sep 11 2017, 4:27 PM
sanjoy requested changes to D35439: Make promoteLoopAccessesToScalars independent of AliasSet [NFC].
Sep 11 2017, 12:10 PM

Sep 10 2017

sanjoy accepted D37659: Improve ScalarEvolution::forgetLoop() performance.

This patch LGTM.

Sep 10 2017, 12:38 PM

Sep 9 2017

sanjoy committed rL312876: [SCEV] Re-arrange public and private sections to be contiguous; NFC.
[SCEV] Re-arrange public and private sections to be contiguous; NFC
Sep 9 2017, 8:56 PM
sanjoy updated the diff for D37432: Fix a misleading phrase in the LangRef.
  • account for vector geps
Sep 9 2017, 8:18 PM
sanjoy requested changes to D37591: [LVI] Move LVILatticeVal class to separate header file (NFC)..
Sep 9 2017, 2:04 PM

Sep 8 2017

sanjoy added a reviewer for D37616: [X86] PR34149 Suboptimal codegen for fast minnum and maxnum.: asbirlea.
Sep 8 2017, 3:07 AM

Sep 7 2017

sanjoy added a comment to D37591: [LVI] Move LVILatticeVal class to separate header file (NFC)..

This isn't a "move" right, since you didn't delete the original?

Sep 7 2017, 2:37 PM
sanjoy added a comment to D37265: [SCEV] Ensure ScalarEvolution::createAddRecFromPHIWithCastsImpl properly handles out of range truncations of the start and accum values.

A couple of quick notes:

  1. The SCEV owner (Sanjoy) added a reviewer after this review was given the thumbs up by another reviewer. I am interpreting this addition of a reviewer an an 'FYI' for the added reviewer.
Sep 7 2017, 9:41 AM

Sep 3 2017

sanjoy created D37432: Fix a misleading phrase in the LangRef.
Sep 3 2017, 6:18 PM
sanjoy requested changes to D37419: Teach scalar evolution to handle inttoptr/ptrtoint.

Hi David,

Sep 3 2017, 12:09 PM

Sep 1 2017

sanjoy added a reviewer for D37265: [SCEV] Ensure ScalarEvolution::createAddRecFromPHIWithCastsImpl properly handles out of range truncations of the start and accum values: silviu.baranga.
Sep 1 2017, 4:45 PM

Aug 28 2017

sanjoy added inline comments to D37153: [LoopUnroll] Properly update loop structure in case of successful peeling.
Aug 28 2017, 8:19 PM

Aug 21 2017

sanjoy committed rL311381: Revert "Reapply: [ADCE][Dominators] Teach ADCE to preserve dominators".
Revert "Reapply: [ADCE][Dominators] Teach ADCE to preserve dominators"
Aug 21 2017, 1:40 PM
sanjoy closed D36979: Revert "Reapply: [ADCE][Dominators] Teach ADCE to preserve dominators" by committing rL311381: Revert "Reapply: [ADCE][Dominators] Teach ADCE to preserve dominators".
Aug 21 2017, 1:40 PM
sanjoy created D36979: Revert "Reapply: [ADCE][Dominators] Teach ADCE to preserve dominators".
Aug 21 2017, 1:27 PM

Aug 13 2017

sanjoy added inline comments to D36656: [SCCP] Propagate integer range info for parameters in IPSCCP..
Aug 13 2017, 3:40 PM

Aug 9 2017

sanjoy accepted D36552: [LVI] Fix LVI compile time regression around constantFoldUser().

lgtm

Aug 9 2017, 6:33 PM
sanjoy added inline comments to D36552: [LVI] Fix LVI compile time regression around constantFoldUser().
Aug 9 2017, 4:09 PM