Page MenuHomePhabricator

sanjoy (Sanjoy Das)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 6 2014, 4:30 PM (254 w, 9 h)

Recent Activity

Thu, Apr 18

sanjoy accepted D60638: [LoopUnroll] Move list of params into a struct [NFCI]..
Thu, Apr 18, 4:04 PM · Restricted Project

Sat, Apr 13

sanjoy added a comment to D60632: [ConstantRange] Disallow NUW | NSW in makeGuaranteedNoWrapRegion().

That was added in D13612 This will be used in a future change to ScalarEvolution.,
but indeed, the (OBO::NoSignedWrap | OBO::NoUnsignedWrap) combination is not used anywhere.
I'm wondering if @sanjoy has any comments?

Sat, Apr 13, 11:00 AM · Restricted Project

Wed, Apr 10

sanjoy accepted D60144: [SCEV] Add option to forget everything in SCEV..

lgtm

Wed, Apr 10, 7:12 PM · Restricted Project

Thu, Apr 4

sanjoy added a comment to D60144: [SCEV] Add option to forget everything in SCEV..

Here are the times I got for compiling the aggregated bitcode files for clang and a few others. Before is ToT, After is with -forget-scev-loop-unroll=true
File Before (s) After (s)
clang-9.bc 7267.91 6639.14
llvm-as.bc 194.12 194.12
llvm-dis.bc 62.50 62.50
opt.bc 1855.85 1857.53

The current test I have is not publishable, I'm still trying to get one.
In the mean time, the above results might motivate having this enabled by default.

Thu, Apr 4, 11:20 AM · Restricted Project
sanjoy added a comment to D59065: [BasicAA] Simplify inttoptr(and(ptrtoint(X), C)) to X, if C preserves all significant bits..

Ah thanks, I was missing the global nature of physical pointers. I couldn't find this described anywhere (besides some of those things mentioned at a tutorial at EuroLLVM). If this is not described anywhere, do you think it would make sense to add it to the AliasAnalysis documentation page, for example?

Thu, Apr 4, 11:06 AM · Restricted Project

Tue, Apr 2

sanjoy added a comment to D59065: [BasicAA] Simplify inttoptr(and(ptrtoint(X), C)) to X, if C preserves all significant bits..

Ah thanks, together with @aqjune 's response, I think I now know what I was missing. If we have something like

int8_t* obj1 = malloc(4);
int8_t* obj2 = malloc(4);
int p = (intptr_t)(obj1 + 4);
 
if (p != (intptr_t) obj2) return;
  
*(int8_t*)(intptr_t)(obj1 + 4) = 0;   // <- here we alias ob1 and obj2?

I thought the information obtained via the control flow, p aliases both obj1 and obj2, is limited to the uses of p, but do I understand correctly that this is not the case and the information leaks to all equivalent expressions (that is for the snippet above, without GVN or any common code elimination)?

Tue, Apr 2, 7:52 PM · Restricted Project
sanjoy added a comment to D59065: [BasicAA] Simplify inttoptr(and(ptrtoint(X), C)) to X, if C preserves all significant bits..

@sanjoy I haven't tried to solve this problem myself, but it seems pretty important. It sounds to me like you're laying out an argument for introducing LLVM pointer masking intrinsics that would preserve some sort of inbound property. Is it fair to say that we probably can't fully optimize tagged pointers without using intrinsics to avoid this ptrtoint optimizer trap?

I think your understanding is correct. To support full optimization opportunity, an intrinsic like llvm.ptrmask(p, mask) would work.

Tue, Apr 2, 7:31 PM · Restricted Project

Mon, Apr 1

sanjoy removed a reviewer for D60078: [FunctionAttrs] Remove old "no-capture" and memory behavior argument deduction: sanjoy.
Mon, Apr 1, 11:10 AM · Restricted Project
sanjoy removed a reviewer for D58311: [MemorySSA & LoopPassManager] Enable MemorySSA as loop dependency. Update tests.: sanjoy.
Mon, Apr 1, 11:10 AM · Restricted Project
sanjoy removed a reviewer for D59903: [NFC][FnAttrs] Stress tests for attribute deduction: sanjoy.
Mon, Apr 1, 11:10 AM · Restricted Project
sanjoy removed a reviewer for D60077: [Attributor] Deduce memory location function attributes: sanjoy.
Mon, Apr 1, 11:07 AM · Restricted Project
sanjoy removed a reviewer for D60076: [Attributor] Deduce memory behavior function attributes: sanjoy.
Mon, Apr 1, 11:07 AM · Restricted Project, Restricted Project
sanjoy removed a reviewer for D60075: [FunctionAttrs] Remove post order "no-recurse" attribute deduction: sanjoy.
Mon, Apr 1, 11:06 AM · Restricted Project
sanjoy removed a reviewer for D60074: [Attributor] Deduce "no-recurse" function attribute: sanjoy.
Mon, Apr 1, 11:06 AM · Restricted Project
sanjoy removed a reviewer for D60073: [Attributor] Enable backwards propagation from callback callees.: sanjoy.
Mon, Apr 1, 11:06 AM · Restricted Project
sanjoy removed a reviewer for D60072: [Attributor] Deduce "no-capture" attributes for call site arguments: sanjoy.
Mon, Apr 1, 11:06 AM · Restricted Project
sanjoy removed a reviewer for D59980: [Attributor] Deduce memory behavior argument attributes: sanjoy.
Mon, Apr 1, 11:06 AM · Restricted Project, Restricted Project
sanjoy removed a reviewer for D59978: [Attributor] Deduce "no-return" function attribute: sanjoy.
Mon, Apr 1, 11:06 AM · Restricted Project
sanjoy removed a reviewer for D59922: [Attributor] Deduce "no-capture" argument attribute: sanjoy.
Mon, Apr 1, 11:06 AM · Restricted Project, Restricted Project
sanjoy removed a reviewer for D59920: [FunctionAttrs] Remove old "returned" argument deduction: sanjoy.
Mon, Apr 1, 11:06 AM · Restricted Project
sanjoy removed a reviewer for D59919: [Attributor] Deduce "returned" argument attribute: sanjoy.
Mon, Apr 1, 11:06 AM · Restricted Project, Restricted Project

Sun, Mar 31

sanjoy added a comment to D59065: [BasicAA] Simplify inttoptr(and(ptrtoint(X), C)) to X, if C preserves all significant bits..

I'm not sure about this. You could have:

int32* ptr0 = malloc(4);
int32* ptr1 = malloc(4);

if (ptr0+1 != ptr1) return;

int32* ptr = (int*)(int64)(ptr0+1);

in which ptr would alias ptr1. But if you transform ptr to ptr0+1 then it would not alias ptr1.

I am probably missing something, but I am not sure how such a case would be possible with this patch? It specifically looks for a inttoptr(and(ptrtoint, C)) sequence, where C is such that the (logical) destination of the pointer remains unchanged. Unfortunately I do not think the LangRef is clear about 'irrelevant' bits in pointers (due to alignment or address spaces)

Sun, Mar 31, 6:51 PM · Restricted Project
sanjoy removed a reviewer for D58560: [AST] Set 'MayAlias' instead of 'MustAlias' in AliasSets for PHI nodes (bugzilla bug#36801): sanjoy.
Sun, Mar 31, 10:35 AM · Restricted Project
sanjoy removed a reviewer for D59020: [StackMaps] Update llvm-readobj to parse V3 Stackmaps: sanjoy.
Sun, Mar 31, 10:35 AM · Restricted Project
sanjoy requested changes to D59065: [BasicAA] Simplify inttoptr(and(ptrtoint(X), C)) to X, if C preserves all significant bits..
Sun, Mar 31, 10:35 AM · Restricted Project
sanjoy requested changes to D60047: [CaptureTracking] Pointer comparisons cannot escape.

Things that are UB in C++ are not necessarily UB in LLVM. For instance you could capture a pointer by doing something like this:

Sun, Mar 31, 10:29 AM · Restricted Project

Fri, Mar 29

sanjoy added a comment to D59065: [BasicAA] Simplify inttoptr(and(ptrtoint(X), C)) to X, if C preserves all significant bits..

I am also not entirely sure how control dependencies could add new underlying objects with this patch

Please read this https://bugs.llvm.org/show_bug.cgi?id=34548

I *think* that this is okay if the ptrtoint and the and have only one user, and they're in the same basic block, and there's nothing that can throw, etc. in between?

Fri, Mar 29, 6:28 PM · Restricted Project
sanjoy committed rG53a5952a9318: Try to fix buildbot error (authored by sanjoy).
Try to fix buildbot error
Fri, Mar 29, 3:26 PM
sanjoy committed rL357324: Try to fix buildbot error.
Try to fix buildbot error
Fri, Mar 29, 3:26 PM
sanjoy committed rG32fd32bc6f6d: [SCEV] Check the cache in get{S|U}MaxExpr before doing any work (authored by sanjoy).
[SCEV] Check the cache in get{S|U}MaxExpr before doing any work
Fri, Mar 29, 3:02 PM
sanjoy committed rL357320: [SCEV] Check the cache in get{S|U}MaxExpr before doing any work.
[SCEV] Check the cache in get{S|U}MaxExpr before doing any work
Fri, Mar 29, 3:01 PM
sanjoy closed D60010: [SCEV] Check the cache in get{S|U}MaxExpr before doing any work.
Fri, Mar 29, 3:01 PM · Restricted Project
sanjoy updated the diff for D60010: [SCEV] Check the cache in get{S|U}MaxExpr before doing any work.

Fix bug: when trying to look up the smax/umax SCEV again (after simplification) we can't use the older ID since it was computed based on the unsimplified operands.

Fri, Mar 29, 2:26 PM · Restricted Project
sanjoy planned changes to D60010: [SCEV] Check the cache in get{S|U}MaxExpr before doing any work.

Sorry, this is buggy, one moment.

Fri, Mar 29, 1:54 PM · Restricted Project
sanjoy created D60010: [SCEV] Check the cache in get{S|U}MaxExpr before doing any work.
Fri, Mar 29, 1:47 PM · Restricted Project

Mar 20 2019

sanjoy added inline comments to D59500: [ConstantFolding] Fix GetConstantFoldFPValue to avoid cast overflow..
Mar 20 2019, 12:20 AM · Restricted Project

Mar 19 2019

sanjoy accepted D59500: [ConstantFolding] Fix GetConstantFoldFPValue to avoid cast overflow..

Lgtm

Mar 19 2019, 6:29 PM · Restricted Project

Mar 18 2019

sanjoy requested changes to D59500: [ConstantFolding] Fix GetConstantFoldFPValue to avoid cast overflow..

This change needs a test, as Roman said.

Mar 18 2019, 1:24 PM · Restricted Project

Mar 16 2019

sanjoy accepted D57428: [SCEV] Guard movement of insertion point for loop-invariants (take 2).

Ok, looks like we're somewhat sloppy with our choice of insertion points here (and it should not be incumbent on you to fix this). Can you please add a comment that mentions that this is due to indvarsimplify?

Mar 16 2019, 2:04 PM · Restricted Project
sanjoy added a comment to D59335: [RFC] Enable vectorization on Neon even without fast-math.

I'm curious as to how will we generate these flags.

Is this the responsibility of the front-end, based on command line options, language standards, target-specific?

Mar 16 2019, 12:39 PM · Restricted Project

Mar 14 2019

sanjoy added a comment to D57428: [SCEV] Guard movement of insertion point for loop-invariants (take 2).

Apologies for the late review Warren.

Mar 14 2019, 9:17 PM · Restricted Project

Mar 13 2019

sanjoy added a comment to D59335: [RFC] Enable vectorization on Neon even without fast-math.

langref part seems to be missing.

Mar 13 2019, 3:49 PM · Restricted Project
sanjoy created D59335: [RFC] Enable vectorization on Neon even without fast-math.
Mar 13 2019, 3:11 PM · Restricted Project

Mar 11 2019

sanjoy accepted D58994: [SCEV] Use depth limit for trunc analysis.

lgtm!

Mar 11 2019, 9:32 PM · Restricted Project
sanjoy committed rG3f5ce18658f0: Reland "Relax constraints for reduction vectorization" (authored by sanjoy).
Reland "Relax constraints for reduction vectorization"
Mar 11 2019, 6:32 PM
sanjoy committed rL355889: Reland "Relax constraints for reduction vectorization".
Reland "Relax constraints for reduction vectorization"
Mar 11 2019, 6:32 PM
sanjoy committed rG2136a5bc49bf: Revert "Relax constraints for reduction vectorization" (authored by sanjoy).
Revert "Relax constraints for reduction vectorization"
Mar 11 2019, 3:37 PM
sanjoy committed rL355873: Revert "Relax constraints for reduction vectorization".
Revert "Relax constraints for reduction vectorization"
Mar 11 2019, 3:37 PM
sanjoy committed rG93f8cc186ace: Relax constraints for reduction vectorization (authored by sanjoy).
Relax constraints for reduction vectorization
Mar 11 2019, 2:36 PM
sanjoy committed rL355868: Relax constraints for reduction vectorization.
Relax constraints for reduction vectorization
Mar 11 2019, 2:35 PM
sanjoy closed D57728: Relax constraints for reduction vectorization.
Mar 11 2019, 2:35 PM · Restricted Project
sanjoy added inline comments to D57728: Relax constraints for reduction vectorization.
Mar 11 2019, 2:27 PM · Restricted Project

Mar 6 2019

sanjoy added inline comments to D57728: Relax constraints for reduction vectorization.
Mar 6 2019, 2:14 PM · Restricted Project
sanjoy updated the diff for D57728: Relax constraints for reduction vectorization.
  • Address review comments
Mar 6 2019, 2:14 PM · Restricted Project

Mar 5 2019

sanjoy updated the summary of D57728: Relax constraints for reduction vectorization.
Mar 5 2019, 3:38 PM · Restricted Project
sanjoy added a reviewer for D57728: Relax constraints for reduction vectorization: Ayal.
Mar 5 2019, 3:36 PM · Restricted Project

Mar 4 2019

sanjoy updated the diff for D57728: Relax constraints for reduction vectorization.

Rebase on trunk.

Mar 4 2019, 8:16 PM · Restricted Project
sanjoy committed rG719e78631de3: PHI nodes are not `FPMathOperator` s (authored by sanjoy).
PHI nodes are not `FPMathOperator` s
Mar 4 2019, 5:15 PM
sanjoy committed rL355362: PHI nodes are not `FPMathOperator` s.
PHI nodes are not `FPMathOperator` s
Mar 4 2019, 5:14 PM
sanjoy closed D58887: PHI nodes are not `FPMathOperator` s.
Mar 4 2019, 5:14 PM · Restricted Project
sanjoy updated the diff for D58887: PHI nodes are not `FPMathOperator` s.

Matt, I noticed a necessary change I had to make before landing this separately.
I didn't notice this because I was initially testing this combined with some
other changes.

Mar 4 2019, 2:14 PM · Restricted Project
sanjoy updated the diff for D58887: PHI nodes are not `FPMathOperator` s.

Add unit test

Mar 4 2019, 11:39 AM · Restricted Project
sanjoy added a comment to D58853: [SCEV] Handle case where MaxBECount is less precise than ExactBECount for OR..

Thanks! While looking at the assertion failure, I also had a look around to see if callers of getBackedgeTakenCount & co check if the returned SCEVExpr is undef, but it seems most users do not do that. I do not have a good concrete example, where using an undef exit count is problematic, but I was wondering if it is safe in general? To me it seems problematic, e.g. if a pass expands an undef SCEVExpr.

One fishy example I could come up with are alias checks in loop-vectorize. If the trip count is undef, we will generate alias checks using undef as upper bound. Those checks could pass (-> say noalias), even though the pointers actually alias in the vector loop ( because we pick a different value for the undef trip count and now the actual trip count is different than the one used for alias checks)

Mar 4 2019, 11:26 AM · Restricted Project
sanjoy accepted D58435: [SCEV] Ensure that isHighCostExpansion takes into account what is being divided.

Thanks for the explanation!

Mar 4 2019, 11:12 AM · Restricted Project
sanjoy added a comment to D58887: PHI nodes are not `FPMathOperator` s.

Test case?

Mar 4 2019, 10:59 AM · Restricted Project

Mar 3 2019

sanjoy added a parent revision for D57728: Relax constraints for reduction vectorization: D58887: PHI nodes are not `FPMathOperator` s.
Mar 3 2019, 2:02 PM · Restricted Project
sanjoy added a child revision for D58887: PHI nodes are not `FPMathOperator` s: D57728: Relax constraints for reduction vectorization.
Mar 3 2019, 2:02 PM · Restricted Project
sanjoy updated the diff for D57728: Relax constraints for reduction vectorization.

Propagate only correct FP fast-math flags.

Mar 3 2019, 2:00 PM · Restricted Project
sanjoy created D58887: PHI nodes are not `FPMathOperator` s.
Mar 3 2019, 1:59 PM · Restricted Project

Mar 1 2019

sanjoy requested changes to D58435: [SCEV] Ensure that isHighCostExpansion takes into account what is being divided.

The change itself looks fine to me, but I'm a bit worried about the test changes. Is it possible that the tests were checking certain the correctness for certain expressions that we no longer check because these happen to be expensive? If that's the case maybe we should add a debug flag that disregards the "is expensive expansion" check to make sure the the same code paths remain tested?

Mar 1 2019, 4:49 PM · Restricted Project
sanjoy accepted D58853: [SCEV] Handle case where MaxBECount is less precise than ExactBECount for OR..

lgtm!

Mar 1 2019, 4:43 PM · Restricted Project
sanjoy accepted D58851: [SCEV] Remove undef check for SCEVConstant (NFC).

lgtm

Mar 1 2019, 4:41 PM · Restricted Project

Feb 19 2019

sanjoy added a reviewer for D57728: Relax constraints for reduction vectorization: kristof.beyls.
Feb 19 2019, 2:36 PM · Restricted Project

Feb 11 2019

sanjoy added a comment to D57728: Relax constraints for reduction vectorization.

ping!

Feb 11 2019, 3:44 PM · Restricted Project

Feb 6 2019

sanjoy accepted D56625: [LICM/MSSA] Add promotion to scalars by building an AliasSetTracker with MemorySSA..

lgtm

Feb 6 2019, 12:02 PM · Restricted Project

Feb 4 2019

sanjoy updated subscribers of D57728: Relax constraints for reduction vectorization.
Feb 4 2019, 5:35 PM · Restricted Project
sanjoy created D57728: Relax constraints for reduction vectorization.
Feb 4 2019, 5:35 PM · Restricted Project

Feb 1 2019

sanjoy requested changes to D57578: [SCEV][WIP] Simplify Add and Mul expressions with Undef.

I don't think we can fold undef within SCEV since SCEV's choice for undef might not be what other transformations choose. For instance if you have a loop:

Feb 1 2019, 9:27 PM
sanjoy accepted D57568: [SCEV] Don't bother preserving LCSSA in SCEV.

I'm okay with the direction, but I'm also worried that this breaks some subtle invariant between SCEV and SCEVExpander. Have you tried bootstrapping clang with this change to see if that shakes out any issues?

Feb 1 2019, 9:06 PM
sanjoy accepted D57567: [SCEV] Do not bother creating separate SCEVUnknown for unreachable nodes.

This is fine, but IMO mapping them to undef feels cleaner conceptually.

Feb 1 2019, 9:04 PM · Restricted Project

Jan 25 2019

sanjoy added inline comments to D56625: [LICM/MSSA] Add promotion to scalars by building an AliasSetTracker with MemorySSA..
Jan 25 2019, 2:21 PM · Restricted Project
sanjoy accepted D57253: [WarnMissedTransforms] Set default to 1..

lgtm

Jan 25 2019, 12:18 PM

Jan 24 2019

sanjoy added inline comments to D35990: [SCEV] Prohibit SCEV transformations for huge SCEVs.
Jan 24 2019, 9:42 AM

Jan 21 2019

sanjoy added a comment to D35989: [SCEV][NFC] Introduces expression sizes estimation.

Re-requesting review per Daniil's comment above. @sanjoy , is this overhead acceptable? I can come up with an alternative maps-based solution if it is not (i.e. only calculate this size for those SCEVs where we need it).

Do you have an estimate of what is the max memory consumption when bootstrapping? I'm pretty sure 0.8M is a very small fraction of the total memory consumption, but would be nice to be sure.

We were estimating max amount of SCEVs that exist at the same time during a bootstrap compilation

Jan 21 2019, 12:28 PM

Jan 16 2019

sanjoy added a comment to D35989: [SCEV][NFC] Introduces expression sizes estimation.

Re-requesting review per Daniil's comment above. @sanjoy , is this overhead acceptable? I can come up with an alternative maps-based solution if it is not (i.e. only calculate this size for those SCEVs where we need it).

Jan 16 2019, 11:07 AM

Jan 10 2019

sanjoy committed rL350886: Avoid use-after-free in ~LegacyRTDyldObjectLinkingLayer.
Avoid use-after-free in ~LegacyRTDyldObjectLinkingLayer
Jan 10 2019, 12:16 PM
sanjoy closed D56521: Avoid use-after-free in ~LegacyRTDyldObjectLinkingLayer.
Jan 10 2019, 12:16 PM
sanjoy added a reviewer for D56521: Avoid use-after-free in ~LegacyRTDyldObjectLinkingLayer: dblaikie.
Jan 10 2019, 11:21 AM

Jan 9 2019

sanjoy accepted D40375: Use MemorySSA in LICM to do sinking and hoisting..

lgtm

Jan 9 2019, 4:41 PM
sanjoy created D56521: Avoid use-after-free in ~LegacyRTDyldObjectLinkingLayer.
Jan 9 2019, 4:25 PM
sanjoy requested changes to D40375: Use MemorySSA in LICM to do sinking and hoisting..

I didn't carefully review the MSSA specific stuff since you're the domain expert here. I do have some general comments.

Jan 9 2019, 1:54 PM

Oct 22 2018

sanjoy updated subscribers of D53306: [X86] Stop promoting integer loads to vXi64.

Hi Craig,

Oct 22 2018, 12:49 PM

Oct 15 2018

sanjoy accepted D53282: [SCEV] Limit AddRec "simplifications" to avoid combinatorial explosions.

lgtm

Oct 15 2018, 7:56 AM

Oct 10 2018

sanjoy accepted D53061: [IndVars] Drop "exact" flag from lshr and udiv when substituting their args.

lgtm

Oct 10 2018, 11:24 AM

Oct 8 2018

sanjoy added inline comments to D51207: Introduce llvm.experimental.widenable_condition intrinsic.
Oct 8 2018, 10:54 AM

Oct 5 2018

sanjoy added a comment to D52930: [SCEV][NFC] Verify IR in isLoop[Entry,Backedge]GuardedByCond.

+1 on the general idea, though like Florian I too think this is too expensive for a normal debug build. How about putting it under a flag (putting under EXPENSIVE_CHECKS requires a recompile right?) that can even run on release builds? That flag can even be enabled by default on EXPENSIVE_CHECKS builds.

Oct 5 2018, 9:58 AM

Sep 24 2018

sanjoy added a comment to D51664: [IR] Lazily number instructions for local dominance queries.

If I get no results, at least 1% RSS is an upper bound on increased LTO memory usage. I'm happy to trade that for 40% shorter compile time of the slowest TUs in clang.

Sep 24 2018, 2:29 PM · Restricted Project
sanjoy added a comment to D51664: [IR] Lazily number instructions for local dominance queries.

The downside is that Instruction grows from 56 bytes to 64 bytes, and I don't have a good way to measure what that costs in practice.

Sep 24 2018, 11:49 AM · Restricted Project

Sep 22 2018

sanjoy added inline comments to D51207: Introduce llvm.experimental.widenable_condition intrinsic.
Sep 22 2018, 11:44 AM

Sep 10 2018

sanjoy added inline comments to D51207: Introduce llvm.experimental.widenable_condition intrinsic.
Sep 10 2018, 2:37 PM