sanjoy (Sanjoy Das)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 6 2014, 4:30 PM (159 w, 1 d)

Recent Activity

Yesterday

sanjoy accepted D33110: [CodeGenPrepare] Don't create inttoptr for ni ptrs.

lgtm

Sat, Jun 24, 9:41 PM
sanjoy accepted D32978: [SCEV] Avoid copying ConstantRange just to get the min/max value.

I did not carefully check the getSignedRange(RHS).getSignedMin() -> getSignedRangeMin(RHS) -like transforms, I assumed you got those right.

Sat, Jun 24, 1:21 PM
sanjoy accepted D31954: [InstCombine] Retain TBAA when narrowing loads.

lgtm

Sat, Jun 24, 12:48 PM
sanjoy resigned from D22298: [LCG] Update and expand comments to properly document the design motivation, tradeoffs, and constraints..

Inactive, as far as I can tell.

Sat, Jun 24, 12:41 PM
sanjoy resigned from D17203: [LICM] Sink entire inner loops..

Inactive, as far as I can tell.

Sat, Jun 24, 12:41 PM
sanjoy resigned from D21041: [GVN] PRE can't hoist loads across calls in general..

Inactive, as far as I can tell.

Sat, Jun 24, 12:38 PM
sanjoy resigned from D21010: Replace the implementation of ConstantRange::binaryAnd..

Inactive, as far as I can tell.

Sat, Jun 24, 12:38 PM
sanjoy resigned from D29965: PR18606 fix hang in GetMinTrailingZeros.

Inactive, as far as I can tell.

Sat, Jun 24, 12:37 PM
sanjoy requested changes to D33332: Possible typo in ProgrammersManual documentation.

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

Sat, Jun 24, 12:37 PM

Thu, Jun 22

sanjoy abandoned D33300: [ImplicitNullChecks] Uphold an invariant in areMemoryOpsAliased.

@skatkov has checked in a fix for this issue.

Thu, Jun 22, 11:53 PM

Wed, Jun 21

sanjoy added inline comments to D33850: Inlining: Don't re-map simplified cloned instructions..
Wed, Jun 21, 11:24 PM

Tue, Jun 20

sanjoy accepted D34385: [ImplicitNullChecks] Uphold an invariant in areMemoryOpsAliased.

lgtm

Tue, Jun 20, 10:35 PM
sanjoy accepted D34397: [SCEV] Make MulOpsInlineThreshold lower to avoid excessive compilation time.

What about the old TODO item of extending powi to integer types and folding them together in the middle end?

Anyways, even if we have a concept of powi in IR, it does not automatically mean that we have it in SCEV. It would be extremely good to have it, though, but it looks like a massive task that takes time.

Tue, Jun 20, 10:29 PM
sanjoy added a comment to D34397: [SCEV] Make MulOpsInlineThreshold lower to avoid excessive compilation time.

32 seems like a sane default value, but where exactly do we get stuck here? If there are O(N^2) or O(N^3) algorithms that are not super difficult to remove, we should just remove them instead of working around them like this.

Tue, Jun 20, 10:28 PM
sanjoy requested changes to D34207: [IndVarSimplify] Add AShr exact flags using induction variables ranges..
Tue, Jun 20, 10:03 AM

Mon, Jun 19

sanjoy added a comment to D33865: Mark llvm.*annotation intrinsics as NoMem and Speculatable.

Do we need to add more reviewers?

JFYI: so far you haven't added any reviewers. :)

However, if you want to make this change, at the very least you also need to edit the language reference entry for llvm.var.annotation to state that it isn't allowed to express control flow dependent properties. Can you also please give some examples of how you use llvm.var.annotate (for discussion)?

I use llvm.annotation to attach an integer number to some loads and stores to identify them uniquely. For loads, I annotate the return value, i.e. llvm.annotation(load(pointer), number). For stores, I annotate the input value being stored, i.e. store(llvm.annotation(value, number)). The load and store offsets can be rather arbitrary and not meaningful in my case (graphics shaders), but the numbers are unique. Then I run LLVM IR optimizations (DCE, CSE, etc.), and then I walk the IR and look at llvm.annotation(.., number) to see which loads and stores survived DCE. Since the loads and stores are from "invisible" memory, CSE can be used, but I need IntrNoMem and IntrSpeculatable there, so that llvm.annotation itself doesn't prevent CSE from being done.

Mon, Jun 19, 10:19 PM
sanjoy committed rL305756: Fix machine instruction in test case.
Fix machine instruction in test case
Mon, Jun 19, 3:36 PM
sanjoy updated the diff for D30445: [ValueTracking] Add a isIVNeverPoison helper.
Mon, Jun 19, 11:30 AM

Sat, Jun 17

sanjoy accepted D34323: Add ArgMemOnly attribute to strlen and wcslen, i.e. they only read memory (string) passed to them..

lgtm

Sat, Jun 17, 8:00 PM
sanjoy requested changes to D34323: Add ArgMemOnly attribute to strlen and wcslen, i.e. they only read memory (string) passed to them..

Generally looks good, but the test isn't testing anything.

Sat, Jun 17, 7:09 PM
sanjoy added inline comments to D30446: [IndVars] Do not branch on poison.
Sat, Jun 17, 3:45 PM
sanjoy added inline comments to D30445: [ValueTracking] Add a isIVNeverPoison helper.
Sat, Jun 17, 3:17 PM
sanjoy added inline comments to D30443: Add a propagateKnownNonPoison helper.
Sat, Jun 17, 3:17 PM
sanjoy committed rL305639: [SROA] Add support for non-integral pointers.
[SROA] Add support for non-integral pointers
Sat, Jun 17, 1:28 PM
sanjoy closed D32203: [SROA] Add support for non-integral pointers by committing rL305639: [SROA] Add support for non-integral pointers.
Sat, Jun 17, 1:28 PM
sanjoy added a comment to D33865: Mark llvm.*annotation intrinsics as NoMem and Speculatable.

Do we need to add more reviewers?

Sat, Jun 17, 1:28 PM

Fri, Jun 16

sanjoy accepted D34025: [SCEV] Teach SCEVExpander to expand BinPow.

lgtm with nits

Fri, Jun 16, 9:04 PM

Wed, Jun 14

sanjoy accepted D33984: [ScalarEvolution] Apply Depth limit to getMulExpr.

lgtm

Wed, Jun 14, 11:04 PM

Sun, Jun 11

sanjoy accepted D33466: Fix file permissions.

You're just making the file non-executable, right? If so, LGTM!

Sun, Jun 11, 9:56 PM
sanjoy added a comment to D33839: Prevent outlining of basicblock that uses BlockAddress.

The spec probably needs update, but I remember inliner or function cloning does something to avoid this. +cc Chandler for comments.

Sun, Jun 11, 9:48 PM

Sat, Jun 10

sanjoy added a comment to D34025: [SCEV] Teach SCEVExpander to expand BinPow.

Sanjoy, what is the difference between having one SCEV with exponential number of operands or having a SCEV tree with 2 operands in each node, but the same number of operands in total? We won't win anything doing so.

Sat, Jun 10, 12:47 PM

Fri, Jun 9

sanjoy requested changes to D33984: [ScalarEvolution] Apply Depth limit to getMulExpr.
Fri, Jun 9, 2:48 PM
sanjoy requested changes to D34025: [SCEV] Teach SCEVExpander to expand BinPow.

Hi Maxim,

Fri, Jun 9, 2:36 PM

Wed, Jun 7

sanjoy accepted D33979: [IndVars] Add an option to be able to disable LFTR.

This LGTM.

Wed, Jun 7, 11:59 AM

Mon, Jun 5

sanjoy added inline comments to D21723: [RFC] Enhance synchscope representation.
Mon, Jun 5, 1:04 PM

Sun, Jun 4

sanjoy added inline comments to D21723: [RFC] Enhance synchscope representation.
Sun, Jun 4, 9:57 PM
sanjoy accepted D33584: Handle non-unique edges in edge-dominance.

lgtm

Sun, Jun 4, 9:32 PM
sanjoy accepted D33654: [docs] Make it clear shifts yield poison when shift amount >= bitwidth.

This one for example (stole from Dave; I don't have my list here handy):

Sun, Jun 4, 2:50 PM
sanjoy requested changes to D30703: [DSE] Merge stores when the later store only writes to memory locations the early store also wrote to..

Some comments inline.

Sun, Jun 4, 12:46 PM

Mon, May 29

sanjoy added a comment to D33654: [docs] Make it clear shifts yield poison when shift amount >= bitwidth.

A few instcombine optimizations rely on this already.

Mon, May 29, 8:07 PM

Fri, May 26

sanjoy accepted D33617: Rearrange Dom unittest to accommodate multiple tests.

lgtm

Fri, May 26, 6:43 PM
sanjoy added a comment to D33584: Handle non-unique edges in edge-dominance.

I certainly agree that if we're not returning false for the non-unique edge case then that will cause bugs later on.

Can I add unit-tests for edge-domination somehow? I am trying to test this with allowing non-unique edges in GVN but that won't fly as regression test.

Fri, May 26, 3:16 PM
sanjoy added a comment to D33584: Handle non-unique edges in edge-dominance.

Today if the bb0 -> bb1 edge dominates some use of %cond then said use can be replaced with i1 true, but with your change that will no longer hold.

Today, any caller that asks if edge bb0->bb1 dominates some use of cond, it will assert :)

Fri, May 26, 3:09 PM
sanjoy added a comment to D33584: Handle non-unique edges in edge-dominance.

Removed the asserts. As Danny put it:

"Given LLVM defines edge dominance in a way that means non-unique edges never dominate their end, this is a waste of time."

In other words, there is no need for an assert here since it's not the case
that the answer would be wrong or would take more time to compute than for
unique edges. It's simply that the answer would always be non-dominance by
how domininance of edges is defined.

Fri, May 26, 2:22 PM

May 25 2017

sanjoy accepted D33129: [SCEVExpander] Try harder to avoid introducing inttoptr.

lgtm

May 25 2017, 11:03 PM
sanjoy accepted D33543: Re-enable "[SCEV] Do not fold dominated SCEVUnknown into AddRecExpr start".

Sanjoy, what happens is nearly following: without the change, LSR was making an overconfident simplification basing on a wrong SCEV. Apparently it did not need the IV analysis to do this. With the change, it chose a different way to simplify (that wasn't so confident), and this way required the IV analysis.

May 25 2017, 9:49 PM
sanjoy added a comment to D33543: Re-enable "[SCEV] Do not fold dominated SCEVUnknown into AddRecExpr start".

Hi Maxim,

May 25 2017, 5:01 PM

May 23 2017

sanjoy accepted D33316: [SCEV] Do not fold dominated SCEVUnknown into AddRecExpr start.

lgtm with a minor nit

May 23 2017, 5:04 PM
sanjoy added a comment to D30527: Replacing float with new class Fraction for LSR alternative way of resolving complex solution.

As for tests, this change should not change anything. The behavior should be the same. (float are replaced with fixed point to avoid potential different results on different CPUs).

May 23 2017, 3:47 PM
sanjoy requested changes to D33433: [SCEV] Select between two equal values.

I'm not convinced SCEV is the best place to handle this -- by the time IR hits SCEV select A, B, B should already have been simplified to B. We do not handle "simple" patterns like br (xor C, true), label A, label B for the same reason.

May 23 2017, 9:18 AM

May 22 2017

sanjoy added inline comments to D32740: [PM/Unswitch] Fix a bug in the domtree update logic for the new unswitch pass..
May 22 2017, 11:34 PM

May 21 2017

sanjoy committed rL303531: [SCEV] Clarify behavior around max backedge taken count.
[SCEV] Clarify behavior around max backedge taken count
May 21 2017, 11:46 PM
sanjoy added inline comments to D33316: [SCEV] Do not fold dominated SCEVUnknown into AddRecExpr start.
May 21 2017, 10:50 PM
sanjoy added a comment to D30527: Replacing float with new class Fraction for LSR alternative way of resolving complex solution.

Drive by comment: how about putting the FixedPoint64 in ADT and adding one or two unit tests?

May 21 2017, 10:02 PM
sanjoy added a comment to D32737: [Constants][SVE] Represent the runtime length of a scalable vector.

I've only lightly read the spec, but it looks like the vector length can be controlled by writing to the ZCR_ELn registers (so, e.g. user code could make a syscall to change the vector length)?

As I explained to Hal on his comment, that is correct but doesn't have the effect you're expecting.

Vectors don't have length, they have the "idea that they may have length", and it's up to the CPU to control that.

Just to be clear, the example you propose has no effect on the notion of length:

// SVE length defined at boot time to be 4
...
add z0.s, p0/m, z0.s, z1.s // z0+=z1 only where the predicate p0 is valid, which here is "up-to" 4 vector lengths
...
svc ... // Try to change vector length to 8, assuming this works
...
add z0.s, p0/m, z0.s, z1.s // z0+=z1 only where the predicate p0 is valid, which here is "up-to" 8 vector lengths
May 21 2017, 12:40 PM

May 20 2017

sanjoy committed rL303498: Revert "[SCEV] Clarify behavior around max backedge taken count".
Revert "[SCEV] Clarify behavior around max backedge taken count"
May 20 2017, 10:02 PM
sanjoy committed rL303497: [SCEV] Clarify behavior around max backedge taken count.
[SCEV] Clarify behavior around max backedge taken count
May 20 2017, 6:48 PM
sanjoy requested changes to D33129: [SCEVExpander] Try harder to avoid introducing inttoptr.

Thank you for doing this!

May 20 2017, 6:22 PM
sanjoy added a comment to D32737: [Constants][SVE] Represent the runtime length of a scalable vector.

I've only lightly read the spec, but it looks like the vector length can be controlled by writing to the ZCR_ELn registers (so, e.g. user code could make a syscall to change the vector length)? If that's accurate, I think a constant vscale is not sufficient.

May 20 2017, 5:44 PM
sanjoy added a comment to D33125: Introduce isoneof<T0, T1, ...> as an extension of isa<T>.

Another high-level comment: I don't like the name isoneof. To me, that parses to the relationship being tested is rather than isa which loses an important distinction. I'm not really sure what would be the best name though.

May 20 2017, 4:57 PM
sanjoy added a comment to D32737: [Constants][SVE] Represent the runtime length of a scalable vector.

What are the semantics of select when the two vectors have different width? Does store do a memory allocation?

May 20 2017, 4:22 PM

May 19 2017

sanjoy requested changes to D33316: [SCEV] Do not fold dominated SCEVUnknown into AddRecExpr start.
May 19 2017, 5:04 PM
sanjoy accepted D33257: [JumpThreading] Replace uses of Condition safely.

lgtm with nits

May 19 2017, 5:03 PM
sanjoy requested changes to D32740: [PM/Unswitch] Fix a bug in the domtree update logic for the new unswitch pass..

Getting this out of my review queue (I'm still waiting on the "I don't see why non-determinism in the worklist order matters here." bit).

May 19 2017, 4:19 PM
sanjoy added inline comments to D33316: [SCEV] Do not fold dominated SCEVUnknown into AddRecExpr start.
May 19 2017, 12:20 PM

May 18 2017

sanjoy requested changes to D33316: [SCEV] Do not fold dominated SCEVUnknown into AddRecExpr start.

Comments inline.

May 18 2017, 10:59 AM
sanjoy added a comment to D33257: [JumpThreading] Replace uses of Condition safely.

Mostly minor stuff.

May 18 2017, 10:49 AM

May 17 2017

sanjoy added inline comments to D33300: [ImplicitNullChecks] Uphold an invariant in areMemoryOpsAliased.
May 17 2017, 8:05 PM
sanjoy added inline comments to D33300: [ImplicitNullChecks] Uphold an invariant in areMemoryOpsAliased.
May 17 2017, 3:35 PM
sanjoy added inline comments to D33300: [ImplicitNullChecks] Uphold an invariant in areMemoryOpsAliased.
May 17 2017, 3:23 PM
sanjoy created D33300: [ImplicitNullChecks] Uphold an invariant in areMemoryOpsAliased.
May 17 2017, 3:17 PM

May 16 2017

sanjoy added a comment to D32203: [SROA] Add support for non-integral pointers.

ping!

May 16 2017, 5:47 PM
sanjoy added inline comments to D33129: [SCEVExpander] Try harder to avoid introducing inttoptr.
May 16 2017, 4:24 PM
sanjoy requested changes to D33257: [JumpThreading] Replace uses of Condition safely.
May 16 2017, 2:58 PM
sanjoy added inline comments to D33129: [SCEVExpander] Try harder to avoid introducing inttoptr.
May 16 2017, 1:33 PM
sanjoy accepted D33228: [SCEV] Always sort AddRecExprs from different loops by dominance.

lgtm!

May 16 2017, 10:42 AM
sanjoy accepted D33231: [SCEV][NFC] Replace redundant dyn_cast with cast in getAddExpr.

Btw, this would have been okay to land without pre-commit review.

May 16 2017, 8:55 AM

May 15 2017

sanjoy added a comment to D33121: [SCEV] Fix sorting order for AddRecExprs.

I have two more comments:

May 15 2017, 10:24 PM
sanjoy accepted D33121: [SCEV] Fix sorting order for AddRecExprs.

lgtm with one comment.

May 15 2017, 6:05 PM
sanjoy added a comment to D31924: SROA: Allow eliminating addrspacecasted allocas.

There was some discussion about non-integral address spaces at EuroLLVM. The current restriction is too great, as not allowing ptrtoint and inttoptr makes it impossible to support C-like languages. We discussed refining the definition to be that optimisers should not introduce inttoptr or ptrtoint, but that they are allowed to be inserted by the front end in places where they are valid in the context of the source language.

May 15 2017, 9:45 AM

May 14 2017

sanjoy committed rL303032: Move some code into ScalarEvolution.cpp; NFC.
Move some code into ScalarEvolution.cpp; NFC
May 14 2017, 9:35 PM
sanjoy added a comment to D32712: [ConstantRange] Reduce the number of allocations in ConstantRange::makeGuaranteedNoWrapRegion.

Hi Craig,

May 14 2017, 3:05 PM
sanjoy requested changes to D30703: [DSE] Merge stores when the later store only writes to memory locations the early store also wrote to..
May 14 2017, 2:38 PM
sanjoy added inline comments to D30703: [DSE] Merge stores when the later store only writes to memory locations the early store also wrote to..
May 14 2017, 2:38 PM
sanjoy added inline comments to D32614: [GVNHoist] Fix: PR32821, add check for anticipability in case of infinite loops.
May 14 2017, 2:07 PM

May 13 2017

sanjoy updated subscribers of D18738: Add new !unconditionally_dereferenceable load instruction metadata.

@hfinkel Oh, my bad--I now remember that this came up long ago...

May 13 2017, 6:44 PM

May 12 2017

sanjoy added inline comments to D32699: [PM/Unswitch] Teach the new simple loop unswitch to handle loop invariant PHI inputs and to rewrite PHI nodes during the actual unswitching..
May 12 2017, 1:42 PM
sanjoy added inline comments to D32740: [PM/Unswitch] Fix a bug in the domtree update logic for the new unswitch pass..
May 12 2017, 11:49 AM
sanjoy added a comment to D33121: [SCEV] Fix sorting order for AddRecExprs.

Hi Max,

May 12 2017, 11:41 AM
sanjoy added a comment to D33125: Introduce isoneof<T0, T1, ...> as an extension of isa<T>.

Overall, I think this is a really good abstraction, but given the scope of the change I'll wait for other stakeholders to chime in.

May 12 2017, 9:32 AM

May 11 2017

sanjoy added a comment to D33088: [LiveVariables] Switch Kill/Defs sets to be `DenseSet`.

How big does the set get? Did you try using SmallDenseSet instead?

May 11 2017, 5:10 PM
sanjoy added a comment to D18738: Add new !unconditionally_dereferenceable load instruction metadata.

@sanjoy Since D20116 is in, is there any reason to avoid having a !speculatable on load instructions? It can be emulated anyway by defining a class of @load.x functions marked speculatable and their return value dereferenceable, so there is no loss of soundness.

May 11 2017, 10:56 AM

May 8 2017

sanjoy committed rL302482: Add basic test case for -instnamer.
Add basic test case for -instnamer
May 8 2017, 4:32 PM
sanjoy committed rL302480: [InstNamer] Don't check type of arguments (they're never void).
[InstNamer] Don't check type of arguments (they're never void)
May 8 2017, 4:32 PM
sanjoy committed rL302481: [InstNamer] Use range-for.
[InstNamer] Use range-for
May 8 2017, 4:32 PM
sanjoy committed rL302479: Delete trailing whitespace.
Delete trailing whitespace
May 8 2017, 4:31 PM
sanjoy committed rL302456: Add a blurb to the release notes about the WeakVH -> WeakTrackingVH transition.
Add a blurb to the release notes about the WeakVH -> WeakTrackingVH transition
May 8 2017, 12:28 PM
sanjoy added a comment to D32006: Mark invariant.group.barrier as inaccessiblememonly.

Is it possible to add "writeonly"? I am not sure if it will help in any way, but the tests seems to be working it.

Yes, I think this makes sense. The model is that there's some side table holding the object type of all memory, and this barrier represents places where we might be updating that table to assign a different type. Right?

May 8 2017, 8:28 AM

May 7 2017

sanjoy added a comment to D32850: [ArgPromotion] Fix a truncated variable.

If I understand what happened, this is scary -- if clang-tidy introduces bugs like this perhaps we should think twice before checking in large mechanical changes like 273808?

May 7 2017, 1:54 PM
sanjoy added a comment to D32762: [Atomic] Remove IsStore/IsLoad in the interface, and pass the instruction instead. NFC..

Random drive by comment.

May 7 2017, 1:22 PM