Page MenuHomePhabricator

sanjoy.google (Sanjoy Das (Work Account))
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 30 2019, 3:04 PM (63 w, 6 d)

Recent Activity

Thu, Oct 15

sanjoy.google removed a reviewer for D87832: [IndVars] Remove monotonic checks with unknown exit count: sanjoy.google.
Thu, Oct 15, 9:53 PM · Restricted Project
sanjoy.google added a comment to D89324: [mlir][gpu] Allow gpu.launch_func to be async..

The semantics of async or not is *very* different from most analysis point of view. It isn't clear to me that having a single op for modeling both instead of a new operation is the right tradeoff here: I'm afraid it is too error prone.

Thu, Oct 15, 9:50 PM · Restricted Project

Jul 31 2020

sanjoy.google removed a reviewer for D77560: [SCEV] don't try to query getSCEV() for incomplete PHIs: sanjoy.google.
Jul 31 2020, 1:06 PM · Restricted Project
sanjoy.google removed a reviewer for D84399: [SCEVExpander] Avoid re-using existing casts if it means updating users.: sanjoy.google.
Jul 31 2020, 11:52 AM · Restricted Project

Jun 11 2020

sanjoy.google added a comment to D81156: [Support] Add stream tie function and use it for errs().

This is surprising and unsafe behaviour. Can we reconsider changing the default, at least until a more careful audit of the places errs() is used?
In particular, if errs is used from multiple threads (with a lock) and outs is used from a single thread (so no lock needed) then this introduces racy flushes to outs() concurrent with writes.

Thanks for highlighting this, it's been brought up on D81525 as well, so rather than discuss it in two places, we'll look at it there, if that's okay?

Happy to discuss there. It's a separate issue though, the discussion there is about initialization being racy,

Yeah, I realised that after posting that previous post, sorry! Still, probably best to keep the discussion in one place for now.

Did we resolve this? In an internal XLA build tsan complains about this being racy, and I'm not sure if the problem is with how we are using llvm::dbgs() or with this patch. If the latter, please rollback while we figure out a non-racy way to achieve the same thing.

Does 030897523d43e3296f69d25a71a140d9e5793c6a fix the race?

Jun 11 2020, 3:59 PM · Restricted Project
sanjoy.google added a comment to D81156: [Support] Add stream tie function and use it for errs().

This is surprising and unsafe behaviour. Can we reconsider changing the default, at least until a more careful audit of the places errs() is used?
In particular, if errs is used from multiple threads (with a lock) and outs is used from a single thread (so no lock needed) then this introduces racy flushes to outs() concurrent with writes.

Thanks for highlighting this, it's been brought up on D81525 as well, so rather than discuss it in two places, we'll look at it there, if that's okay?

Happy to discuss there. It's a separate issue though, the discussion there is about initialization being racy,

Yeah, I realised that after posting that previous post, sorry! Still, probably best to keep the discussion in one place for now.

Jun 11 2020, 3:26 PM · Restricted Project

Feb 7 2020

sanjoy.google added a comment to D74185: Revert the revert of vectorization commits.

Thanks, will check this.

Should we revert the commit while you investigate?

The problem here that this patch does not introduce new vectorization, instead it just triggers the existing vetorization for more cases. If there is a bug in the vectorizer, this patch just allows to reveal it, not introduces it.

Feb 7 2020, 4:32 PM · Restricted Project
sanjoy.google added a comment to D74185: Revert the revert of vectorization commits.

Thanks, will check this.

Feb 7 2020, 4:05 PM · Restricted Project

Jan 27 2020

sanjoy.google removed a reviewer for D73496: [IRCE] Use SCEVExpander to modify loop bound: sanjoy.google.
Jan 27 2020, 11:54 AM · Restricted Project

Jan 23 2020

sanjoy.google added inline comments to D73181: [SCEV] Use backedge SCEV of PHI only if its input is loop invariant.
Jan 23 2020, 10:09 AM · Restricted Project
sanjoy.google added inline comments to D73181: [SCEV] Use backedge SCEV of PHI only if its input is loop invariant.
Jan 23 2020, 7:34 AM · Restricted Project

Jan 22 2020

sanjoy.google added inline comments to D73181: [SCEV] Use backedge SCEV of PHI only if its input is loop invariant.
Jan 22 2020, 9:09 AM · Restricted Project
sanjoy.google added inline comments to D73181: [SCEV] Use backedge SCEV of PHI only if its input is loop invariant.
Jan 22 2020, 8:14 AM · Restricted Project

Jan 10 2020

sanjoy.google removed a reviewer for D72519: [LoopInfo] Support multi edge in getLoopLatch(): sanjoy.google.
Jan 10 2020, 9:53 AM · Restricted Project

Jan 9 2020

sanjoy.google added a comment to D71690: [SCEV] get a more accurate range for AddRecExpr with nuw flag.

My understanding is that both the unsigned and signed ranges are valid and we are free to use either one, so using the unsigned one is more beneficial in an unsigned context.

Jan 9 2020, 8:12 AM · Restricted Project

Dec 19 2019

sanjoy.google requested changes to D71690: [SCEV] get a more accurate range for AddRecExpr with nuw flag.

This needs a specific test case.

Dec 19 2019, 7:24 AM · Restricted Project

Dec 16 2019

sanjoy.google added a comment to D71563: [SCEV] Recognise the hardwareloop "loop.decrement.reg" intrinsic.

Thanks for taking a look!

llvm.loop.decrement.reg is not documented in the langref. Is this a target specific thing?

No, it's a target independent thing, and is generated by the generic Hardwareloop pass. It is defined here:

https://github.com/llvm/llvm-project/blob/ff07fc66d9eef577f3b44716f72e581a18cd9ac9/llvm/include/llvm/IR/Intrinsics.td#L1304

Also, is the entire gamut of optimizations this enables OK? For instance, would it be okay to LFTR replace the trip count of the loop such that it doesn't use the loop_decrement_reg intrinsic anymore?

Hmmm, I don't have a good answer for that yet. First, just to be clear, the llvm.loop.decrement.reg intrinsic is exactly the same as a sub expression, so I think SCEV analysis functions should be okay. But yes, I think I can imagine that SCEV transformations that modify the loop update expression could potentially be problematic. But this is a bit of unexplored territory for me, so first I need to look into what LFTR exactly does. So, I will need to so some homework first, but if we assume for a moment that there will be problematic cases with SCEV transformation modifying the loop update expression (if they would ignore the loop.decremenent intrinsic if there is one present), does that make this approach completely unfeasible, or would you still see possibilities to use the SCEV analysis functions?

Dec 16 2019, 2:11 PM · Restricted Project
sanjoy.google added a comment to D71563: [SCEV] Recognise the hardwareloop "loop.decrement.reg" intrinsic.

llvm.loop.decrement.reg is not documented in the langref. Is this a target specific thing?

Dec 16 2019, 12:25 PM · Restricted Project
sanjoy.google accepted D70156: [APInt] Fix tests that had wrong assumption about sdivs with negative quotient..

lgtm

Dec 16 2019, 7:20 AM · Restricted Project
sanjoy.google accepted D71537: [SCEV] Move ScalarEvolutionExpander.cpp to Transforms/Utils (NFC)..

lgtm

Dec 16 2019, 7:20 AM · Restricted Project

Oct 7 2019

sanjoy.google added a comment to D68194: [LCSSA] Forget values we create LCSSA phis for.

If the SCEV expression is mathematically correct I think this is a problem with SCEV expander. If it is expected to preserve LCSSA it should add the necessary PHIs.

Oct 7 2019, 2:21 PM · Restricted Project
sanjoy.google accepted D68592: [SCEV] Add stricter verification option..

Last time I looked at this I concluded it would be difficult to make enable something like this consistently, but no harm in trying.

Oct 7 2019, 2:16 PM · Restricted Project

Sep 12 2019

sanjoy.google accepted D67417: [LICM/AST] Check if the AliasAny set is removed from the tracker..

lgtm

Sep 12 2019, 10:58 AM · Restricted Project

Sep 4 2019

sanjoy.google accepted D67177: [SCEV] Support SCEVUMinExpr in getRangeRef..

lgtm

Sep 4 2019, 9:27 AM · Restricted Project

Aug 1 2019

sanjoy.google accepted D65596: [SimplifyCFG] Cleanup redundant conditions [NFC]..

lgtm

Aug 1 2019, 4:24 PM · Restricted Project

Jul 31 2019

sanjoy.google accepted D65417: [SCCP] Update condition to avoid overflow..

lgtm with some nits

Jul 31 2019, 11:01 AM · Restricted Project

Jul 30 2019

sanjoy.google added inline comments to D65417: [SCCP] Update condition to avoid overflow..
Jul 30 2019, 3:11 PM · Restricted Project
sanjoy.google edited reviewers for D65417: [SCCP] Update condition to avoid overflow., added: sanjoy.google; removed: sanjoy.
Jul 30 2019, 3:08 PM · Restricted Project