Page MenuHomePhabricator

Tyker
User

Projects

User does not belong to any projects.

User Details

User Since
Mar 16 2019, 2:09 PM (45 w, 1 d)

Recent Activity

Sat, Jan 25

Tyker added inline comments to D72885: [WIP] Add Query API for llvm.assume holding attributes.
Sat, Jan 25, 12:03 AM · Restricted Project

Fri, Jan 24

Tyker added a parent revision for D73404: [WIP] Basis of droping uses in llvm.assume.: D72885: [WIP] Add Query API for llvm.assume holding attributes.
Fri, Jan 24, 11:54 PM · Restricted Project
Tyker added a child revision for D72885: [WIP] Add Query API for llvm.assume holding attributes: D73404: [WIP] Basis of droping uses in llvm.assume..
Fri, Jan 24, 11:54 PM · Restricted Project
Tyker updated the diff for D73404: [WIP] Basis of droping uses in llvm.assume..
Fri, Jan 24, 11:54 PM · Restricted Project
Tyker added a comment to D72885: [WIP] Add Query API for llvm.assume holding attributes.

I won't review this properly till next week. Generally looks good so you can work on the next patch if you want to.

i started working on ignoring or dropping uses in llvm.assume when a transformation is use sensitive.

Great! Can you share it on phab so I can reference it in my email (see below).

Fri, Jan 24, 11:54 PM · Restricted Project
Tyker created D73404: [WIP] Basis of droping uses in llvm.assume..
Fri, Jan 24, 11:54 PM · Restricted Project

Tue, Jan 21

Tyker added a comment to D72885: [WIP] Add Query API for llvm.assume holding attributes.

I won't review this properly till next week. Generally looks good so you can work on the next patch if you want to.

Tue, Jan 21, 3:45 PM · Restricted Project
Tyker updated the diff for D72884: [NFC] Factor out function to detect if an attribute has an argument..
Tue, Jan 21, 3:27 PM · Restricted Project
Tyker added inline comments to D72475: [WIP] Build assume from call.
Tue, Jan 21, 3:27 PM · Restricted Project

Fri, Jan 17

Tyker added inline comments to D72475: [WIP] Build assume from call.
Fri, Jan 17, 3:39 PM · Restricted Project
Tyker added inline comments to D72885: [WIP] Add Query API for llvm.assume holding attributes.
Fri, Jan 17, 3:39 PM · Restricted Project
Tyker updated the diff for D72475: [WIP] Build assume from call.

fixed.

Fri, Jan 17, 3:39 PM · Restricted Project
Tyker updated the diff for D72885: [WIP] Add Query API for llvm.assume holding attributes.

addressed comments.

Fri, Jan 17, 3:39 PM · Restricted Project

Thu, Jan 16

Tyker added a parent revision for D72885: [WIP] Add Query API for llvm.assume holding attributes: D72884: [NFC] Factor out function to detect if an attribute has an argument..
Thu, Jan 16, 3:24 PM · Restricted Project
Tyker added a child revision for D72884: [NFC] Factor out function to detect if an attribute has an argument.: D72885: [WIP] Add Query API for llvm.assume holding attributes.
Thu, Jan 16, 3:24 PM · Restricted Project
Tyker removed a parent revision for D72884: [NFC] Factor out function to detect if an attribute has an argument.: D72885: [WIP] Add Query API for llvm.assume holding attributes.
Thu, Jan 16, 3:24 PM · Restricted Project
Tyker removed a child revision for D72885: [WIP] Add Query API for llvm.assume holding attributes: D72884: [NFC] Factor out function to detect if an attribute has an argument..
Thu, Jan 16, 3:24 PM · Restricted Project
Tyker added a child revision for D72885: [WIP] Add Query API for llvm.assume holding attributes: D72884: [NFC] Factor out function to detect if an attribute has an argument..
Thu, Jan 16, 3:15 PM · Restricted Project
Tyker added a parent revision for D72884: [NFC] Factor out function to detect if an attribute has an argument.: D72885: [WIP] Add Query API for llvm.assume holding attributes.
Thu, Jan 16, 3:15 PM · Restricted Project
Tyker created D72884: [NFC] Factor out function to detect if an attribute has an argument..
Thu, Jan 16, 3:13 PM · Restricted Project
Tyker added a parent revision for D72884: [NFC] Factor out function to detect if an attribute has an argument.: D72475: [WIP] Build assume from call.
Thu, Jan 16, 3:13 PM · Restricted Project
Tyker created D72885: [WIP] Add Query API for llvm.assume holding attributes.
Thu, Jan 16, 3:13 PM · Restricted Project
Tyker added a child revision for D72475: [WIP] Build assume from call: D72884: [NFC] Factor out function to detect if an attribute has an argument..
Thu, Jan 16, 3:13 PM · Restricted Project
Tyker updated the diff for D72475: [WIP] Build assume from call.

minor improvements.

Thu, Jan 16, 3:13 PM · Restricted Project
Tyker added a comment to D72700: [DSE] Add first version of MemorySSA-backed DSE (Bottom up walk)..

Regarding handling the branching example, the solution fhahn proposes sounds reasonable to me.
I was thinking something similar, along the lines of: do the checks bottom-up from both second and third stores during the main algorithm, both find the dead store but do not postdominate it; don't abandon them, but keep this info (include the checks for no intervening reads) for after the main search. The data structure could be a Hashmap<PotentialDeadStore, ListOfDefsOrBlocksWhoFoundThisPotentialDeadStore>. If all paths are covered in the List for a PotentialDeadStore, then it's truly dead.

Thu, Jan 16, 12:08 PM · Restricted Project

Wed, Jan 15

Tyker added a comment to D72700: [DSE] Add first version of MemorySSA-backed DSE (Bottom up walk)..

I did a few experiment with bottom-up algorithm before the patch i showed on phabricator. my implementation of the bottom-up had similar average complie-time to the current pass on the test-suite but it was only barely removing more stores than the current pass.
i gave up on it because i didn't found a good way forward to make it deal with cases like to the following:

   store i32 0, i32* %Ptr.  ; DEAD
   br i1 %cond, label %a, label %b
a:
  store i32 2, i32* %Ptr
   br label %c
b: 
  store i32 2, i32* %Ptr
  br label %c

which is IMO definitely something we want to do. by the way gcc does this optimization. https://godbolt.org/z/VFBf-c
maybe there is a bottom-up way to deal with this that i didn't thought about. any thought ?

Wed, Jan 15, 4:29 AM · Restricted Project

Tue, Jan 14

Tyker updated the diff for D72475: [WIP] Build assume from call.

i renamed the files to KnowledgeRetention.* to be more general. we can put other related utilities like the query API in the same files. i had a hard time finding a good name and i am not sure i succeeded what do you think ?

Tue, Jan 14, 4:43 PM · Restricted Project
Tyker added inline comments to D72475: [WIP] Build assume from call.
Tue, Jan 14, 4:43 PM · Restricted Project

Mon, Jan 13

Tyker added a comment to D72475: [WIP] Build assume from call.
`bool getAttributeFromAssume(Value &Base, CallInst &AssumeCI, Attribute::Kind Kind, int &Value)`

and similar for string attributes. Alternatively we could return a map from WhatOn values to attributes.

something like that seem fine.

Mon, Jan 13, 10:35 AM · Restricted Project
Tyker added a comment to D63960: [C++20] Add consteval-specific semantic for functions.

@rsmith friendly reminder

Mon, Jan 13, 10:35 AM · Restricted Project

Sun, Jan 12

Tyker updated the diff for D72475: [WIP] Build assume from call.

fixed comments.

Sun, Jan 12, 10:57 AM · Restricted Project

Fri, Jan 10

Tyker added inline comments to D72475: [WIP] Build assume from call.
Fri, Jan 10, 8:57 AM · Restricted Project
Tyker updated the diff for D72475: [WIP] Build assume from call.

fixed comments.

Fri, Jan 10, 8:57 AM · Restricted Project

Thu, Jan 9

Tyker added inline comments to D72475: [WIP] Build assume from call.
Thu, Jan 9, 2:56 PM · Restricted Project
Tyker updated the diff for D72475: [WIP] Build assume from call.

i changed the pass and the test to actually perform the transformation. it is less confusing this way.

Thu, Jan 9, 2:56 PM · Restricted Project
Tyker updated the diff for D72475: [WIP] Build assume from call.
Thu, Jan 9, 1:22 PM · Restricted Project
Tyker added inline comments to D72475: [WIP] Build assume from call.
Thu, Jan 9, 1:22 PM · Restricted Project
Tyker updated the diff for D72475: [WIP] Build assume from call.
Thu, Jan 9, 12:35 PM · Restricted Project
Tyker added a child revision for D72455: [NFC] Refactor TableGen for attributes: D72475: [WIP] Build assume from call.
Thu, Jan 9, 12:35 PM · Restricted Project
Tyker added a parent revision for D72475: [WIP] Build assume from call: D72455: [NFC] Refactor TableGen for attributes.
Thu, Jan 9, 12:35 PM · Restricted Project
Tyker created D72475: [WIP] Build assume from call.
Thu, Jan 9, 12:35 PM · Restricted Project
Tyker created D72455: [NFC] Refactor TableGen for attributes.
Thu, Jan 9, 7:24 AM · Restricted Project

Tue, Jan 7

Tyker updated the diff for D63640: [clang] Improve Serialization/Imporing/Dumping of APValues.

rebased

Tue, Jan 7, 3:42 PM · Restricted Project

Mon, Jan 6

Tyker committed rGf5329bfc76bb: [Diagnostic] make Wmisleading-indendation not warn about labels (authored by Tyker).
[Diagnostic] make Wmisleading-indendation not warn about labels
Mon, Jan 6, 2:26 PM
Tyker closed D72202: [Diagnostic] make Wmisleading-indendation not warn about labels.
Mon, Jan 6, 2:26 PM · Restricted Project
Tyker added inline comments to D72146: [DSE] Add first version of MemorySSA-backed DSE..
Mon, Jan 6, 10:35 AM · Restricted Project

Sun, Jan 5

Tyker added a comment to D72146: [DSE] Add first version of MemorySSA-backed DSE..

Yes, PostDominators is the second thing we need to get better at preserving. IIRC all places where we already use DomTreeUpdater we basically preserve PDT as well without any additional work, besides passing in the PDT to the updater. There's getAnalysisIfAvailable (legacy pass manager)/getCachedResult (new PM) to get an analysis, only if it is already computed. The only caveat is that the legacy pass manager cannot preserve function analysis if it is followed by a function pass (IIUC).

Sun, Jan 5, 2:19 PM · Restricted Project

Sat, Jan 4

Tyker added a comment to D72146: [DSE] Add first version of MemorySSA-backed DSE..

Do you think it would make sense to use D72146 (this patch) as foundation and port the improvements from your patch? It would be great if you could have a look and let me know what you think!

I took a look at this patch and D72148(the patch adding MemPhi traversal). they have many good ideas i haven't thought about.
but i have one major concern.
how to you plan to dealing with store being killed by multiple stores which together post-dominate the "dead" store ?
like the store in the entry block of the following:

define void @test5A(i32* %P) {
  store i32 1, i32* %P ; this is dead
  br i1 true, label %bb1, label %bb2
bb1:
  store i32 0, i32* %P
  br label %bb3
bb2:
  store i32 1, i32* %P
  br label %bb3
bb3:
  ret void
}

the design around getNextMemoryDef seems to me like it is too restrictive for the above example.

Yes as it is it does not cover that case. But I think it would be relatively straight forward to extend it to return a list of MemoryDefs encountered along multiple paths. Those can then be checked for legality separately. If some of the found Defs do not eliminate the original store, we can continue walking just those Defs. To support that, it probably also makes sense to re-write getNextMemoryDef to work iteratively, like in your patch.

Ok seems reasonable, so we will change the core loop for each store will have to change in a following patch.

IMO the most productive way forward would be to get in a patch with limited but solid support as a baseline and once that is in we can work on adding missing functionality in parallel. What do you think about this approach?

you are right. i probably went a bit to far for a first patch.

Besides extending the functionality, I think we will also have to spend a fair amount of work on making sure that preceding and succeeding passes in the pipeline preserve MemorySSA, to avoid computing it from scratch just for DSE.

yeah. and we may have to do the same thing for the post dominator tree.
is there a way to have a pass preserve an analysis if it is still valid without depending on it but not updating it if it is not valid ?

Sat, Jan 4, 2:59 PM · Restricted Project
Tyker updated the diff for D72182: [WIP][DSE] Add basic cross-block dse pass using MemorySSA.

improved the test suite.

Sat, Jan 4, 2:14 PM · Restricted Project
Tyker added inline comments to D72202: [Diagnostic] make Wmisleading-indendation not warn about labels.
Sat, Jan 4, 10:33 AM · Restricted Project
Tyker added a comment to D70638: [Diagnostic] add a warning which warns about misleading indentation.

(re-ping; I think this false positive for goto label case is important to be fixed before 10 release)

I agree -- @Tyker, are you planning to work on that fix?

Sat, Jan 4, 8:44 AM · Restricted Project
Tyker created D72202: [Diagnostic] make Wmisleading-indendation not warn about labels.
Sat, Jan 4, 8:44 AM · Restricted Project
Tyker added inline comments to D72146: [DSE] Add first version of MemorySSA-backed DSE..
Sat, Jan 4, 4:11 AM · Restricted Project
Tyker added a comment to D63960: [C++20] Add consteval-specific semantic for functions.

ping @rsmith

Sat, Jan 4, 3:10 AM · Restricted Project
Tyker added a comment to D72146: [DSE] Add first version of MemorySSA-backed DSE..

Do you think it would make sense to use D72146 (this patch) as foundation and port the improvements from your patch? It would be great if you could have a look and let me know what you think!

Sat, Jan 4, 2:41 AM · Restricted Project

Fri, Jan 3

Tyker committed rGc4766cadcb38: [Diagnostic] Add test for previous b4b904e19bb356724b2c6aea0199ce05c6f15cdb (authored by Tyker).
[Diagnostic] Add test for previous b4b904e19bb356724b2c6aea0199ce05c6f15cdb
Fri, Jan 3, 4:47 PM
Tyker closed D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.

Can you add a test case for that crash? Otherwise OK.

as i said. the bug can only occur for one line files. so we can't have a run line and a the crashing line. so i wasn't able to make a test for it.

Is it possible to have the RUN bit at the end of the line in a comment, after the test code itself?

Fri, Jan 3, 4:47 PM · Restricted Project
Tyker added a comment to D72146: [DSE] Add first version of MemorySSA-backed DSE..

i haven't looked in details at your patch but i was working on something similar D72182.

Fri, Jan 3, 2:17 PM · Restricted Project
Tyker created D72182: [WIP][DSE] Add basic cross-block dse pass using MemorySSA.
Fri, Jan 3, 2:17 PM · Restricted Project
Tyker added a comment to D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.

Can you add a test case for that crash? Otherwise OK.

Fri, Jan 3, 1:01 PM · Restricted Project
Tyker committed rGb4b904e19bb3: [Diagnostic] Fixed add ftabstop to -Wmisleading-indentation (authored by Tyker).
[Diagnostic] Fixed add ftabstop to -Wmisleading-indentation
Fri, Jan 3, 8:23 AM
Tyker updated the diff for D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.

This update should fixes the issue.
the assert was incorrect. because file offsets are 0-based where as column numbers are 1-based.

Fri, Jan 3, 7:35 AM · Restricted Project

Mon, Dec 30

Tyker committed rGb47b35ff51b3: [Diagnostic] Add ftabstop to -Wmisleading-indentation (authored by Tyker).
[Diagnostic] Add ftabstop to -Wmisleading-indentation
Mon, Dec 30, 12:26 AM
Tyker closed D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.
Mon, Dec 30, 12:26 AM · Restricted Project

Dec 22 2019

Tyker added a comment to D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.
Dec 22 2019, 11:45 AM · Restricted Project
Tyker updated the diff for D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.

fixed comments.

Dec 22 2019, 11:40 AM · Restricted Project

Dec 19 2019

Tyker updated the diff for D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.

i agree that it is the best solution.

Dec 19 2019, 9:23 AM · Restricted Project

Dec 18 2019

Tyker added inline comments to D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.
Dec 18 2019, 4:40 AM · Restricted Project
Tyker updated the diff for D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.

my intent was not necessarily to create a generic solution but to get a solution that is sufficient to not have false positives on the Wmisleading-indentation warning for code base that have mixes of tabs and spaces.

Dec 18 2019, 4:40 AM · Restricted Project

Dec 14 2019

Tyker added a comment to D63960: [C++20] Add consteval-specific semantic for functions.

ping @rsmith

Dec 14 2019, 4:23 AM · Restricted Project

Dec 12 2019

Tyker added inline comments to D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.
Dec 12 2019, 1:52 PM · Restricted Project
Tyker updated the diff for D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.
Dec 12 2019, 1:52 PM · Restricted Project
Tyker committed rG9c8cfa09d762: [Diagnsotics] Small Improvement on -Wmisleading-indentation (authored by Tyker).
[Diagnsotics] Small Improvement on -Wmisleading-indentation
Dec 12 2019, 5:56 AM
Tyker closed D71083: [Diagnsotics] Small Improvement on -Wmisleading-indentation.
Dec 12 2019, 5:56 AM · Restricted Project

Dec 10 2019

Tyker added a comment to D63640: [clang] Improve Serialization/Imporing/Dumping of APValues.

now that the issue with uniqueness of expressions is solved. we should be able to keep going on that review @rsmith.
https://reviews.llvm.org/D63960 should be i think close to completion. so maybe for testing i could use immediate invocation as a source for ConstantExpr instead of the code i added to make constexpr variables emit ConstantExpr ?

Dec 10 2019, 1:06 PM · Restricted Project

Dec 9 2019

Tyker added inline comments to D71083: [Diagnsotics] Small Improvement on -Wmisleading-indentation.
Dec 9 2019, 1:30 PM · Restricted Project
Tyker updated the diff for D71083: [Diagnsotics] Small Improvement on -Wmisleading-indentation.
Dec 9 2019, 1:30 PM · Restricted Project

Dec 5 2019

Tyker updated the diff for D63960: [C++20] Add consteval-specific semantic for functions.

comments were fixed.

Dec 5 2019, 2:54 PM · Restricted Project
Tyker added inline comments to D63960: [C++20] Add consteval-specific semantic for functions.
Dec 5 2019, 2:54 PM · Restricted Project
Tyker updated the diff for D71083: [Diagnsotics] Small Improvement on -Wmisleading-indentation.
Dec 5 2019, 11:41 AM · Restricted Project
Tyker added a comment to D70638: [Diagnostic] add a warning which warns about misleading indentation.

i have not tested the performance impact. but i tried to do the heavier checks last to minimize the impact.
i added your tests file to the improvement commit.

Dec 5 2019, 11:21 AM · Restricted Project
Tyker added a parent revision for D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation: D71083: [Diagnsotics] Small Improvement on -Wmisleading-indentation.
Dec 5 2019, 11:11 AM · Restricted Project
Tyker created D71083: [Diagnsotics] Small Improvement on -Wmisleading-indentation.
Dec 5 2019, 11:11 AM · Restricted Project
Tyker added a child revision for D71083: [Diagnsotics] Small Improvement on -Wmisleading-indentation: D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.
Dec 5 2019, 11:11 AM · Restricted Project
Tyker updated the diff for D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.

fixed aaron's comment

Dec 5 2019, 11:11 AM · Restricted Project
Tyker added inline comments to D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.
Dec 5 2019, 11:11 AM · Restricted Project
Tyker added a comment to D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.

i ran a build of the linux kernel using the stage1 compiler, about 30% of files crashed the compiler in the optimizer. everything was building fine 2 weeks ago with the same configuration but a previous compiler.
but aside from that from that file that complied fine, there was only one warning left. this warning is exactly the same case we had in llvm's source code.

Dec 5 2019, 1:53 AM · Restricted Project

Dec 4 2019

Tyker added inline comments to D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.
Dec 4 2019, 3:26 PM · Restricted Project
Tyker updated the diff for D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.
Dec 4 2019, 3:17 PM · Restricted Project
Tyker added a comment to D70638: [Diagnostic] add a warning which warns about misleading indentation.

the warning was not affected by -ftabstop. i made a patch that fix this.

Dec 4 2019, 3:17 PM · Restricted Project
Tyker created D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.
Dec 4 2019, 3:08 PM · Restricted Project
Tyker added a comment to D70638: [Diagnostic] add a warning which warns about misleading indentation.

i did a few test on the linux kernel on prior version of this patchs and the mix of spaces and tabs caused some false positives. but i do believe there is still a bug here.
for the mix of space and tabs. gcc has a -ftabstop=X to specify how large tabs should be counted as.
clang has it as well i am going to check that it is taken in account.

Dec 4 2019, 9:51 AM · Restricted Project

Dec 3 2019

Tyker committed rGbc840b21e161: [Diagnostic] add a warning which warns about misleading indentation (authored by Tyker).
[Diagnostic] add a warning which warns about misleading indentation
Dec 3 2019, 12:25 PM
Tyker committed rG2f9604727526: [NFCI] update formating for misleading indentation warning (authored by Tyker).
[NFCI] update formating for misleading indentation warning
Dec 3 2019, 12:25 PM
Tyker closed D70638: [Diagnostic] add a warning which warns about misleading indentation.
Dec 3 2019, 12:25 PM · Restricted Project
Tyker closed D70861: [NFCI] update formating for misleading indentation warning.
Dec 3 2019, 12:25 PM · Restricted Project
Tyker updated the diff for D70638: [Diagnostic] add a warning which warns about misleading indentation.
Dec 3 2019, 11:10 AM · Restricted Project

Dec 2 2019

Tyker updated the diff for D70638: [Diagnostic] add a warning which warns about misleading indentation.

improved based on aaron's comment.

Dec 2 2019, 3:34 PM · Restricted Project
Tyker committed rG9ec6d7121132: [clang][modules] Add support for merging lifetime-extended temporaries (authored by Tyker).
[clang][modules] Add support for merging lifetime-extended temporaries
Dec 2 2019, 11:02 AM

Dec 1 2019

Tyker committed rGae5484540f15: Revert "[clang][modules] Add support for merging lifetime-extended temporaries" (authored by Tyker).
Revert "[clang][modules] Add support for merging lifetime-extended temporaries"
Dec 1 2019, 1:43 PM