Page MenuHomePhabricator

ekatz (Ehud Katz)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 16 2019, 1:22 AM (49 w, 1 d)

Recent Activity

Jul 20 2020

ekatz added a comment to D80116: [IR] Simplify Use::swap. NFCI..

I just wonder if there is anyone counting on the current ordering of uses.
Although, I think they shouldn't...

Jul 20 2020, 7:29 AM · Restricted Project
ekatz accepted D80116: [IR] Simplify Use::swap. NFCI..

I like it! Great job!

Jul 20 2020, 7:27 AM · Restricted Project

Jun 26 2020

ekatz added inline comments to D81236: Improve LegacyPassManager API to correctly report modified status.
Jun 26 2020, 4:18 AM · Restricted Project

Jun 25 2020

ekatz accepted D81236: Improve LegacyPassManager API to correctly report modified status.

I admit, I don't like this change. This is a work around a design flaw in the old PM. However, it seems like the right decision to handle it, right now.
I think we should post a RFC for a new - *cough* - PM... ;)

Jun 25 2020, 10:13 AM · Restricted Project
ekatz added inline comments to D81236: Improve LegacyPassManager API to correctly report modified status.
Jun 25 2020, 1:03 AM · Restricted Project

Jun 24 2020

ekatz added inline comments to D81236: Improve LegacyPassManager API to correctly report modified status.
Jun 24 2020, 12:28 PM · Restricted Project

Jun 18 2020

ekatz updated subscribers of D81236: Improve LegacyPassManager API to correctly report modified status.

Do you have someone in mind? Or would you just want to wait?

Not sure if waiting would help much, if we don't add new reviewers. Perhaps add @chandlerc , @rnk , @lattner , @mehdi_amini, @asbirlea, and maybe others who worked on these files (if needed).

Jun 18 2020, 8:43 PM · Restricted Project
ekatz requested changes to D81236: Improve LegacyPassManager API to correctly report modified status.

This change should be split into 2.
One for the PM changes and the other for the LoopExtractor (that depend on the previous change).
Regardless, the changes in the PM should be reviewed by more people...

Jun 18 2020, 1:06 PM · Restricted Project

Jun 10 2020

ekatz added a comment to D81236: Improve LegacyPassManager API to correctly report modified status.

I investigated, and BreakCriticalEdges is a dependency of LoopExtractor, and it's the pass that triggers the change.

This makes more sense. Every pass (in our case - BreakCriticalEdges) is responsible for reporting to the PM about the preserved (and non preserved) analyses.
I am not sure, what bug are you trying to tackle?
In case the LoopExtractor does nothing, it will report that all analyses are preserved, but the preceding call to BreakCriticalEdges will (and if it doesn't - it should) already report the non preserved analyses.
I guess, what you didn't expect is the fact that running the LoopExtractor will trigger BreakCriticalEdges, but although it may be a strange/ugly behavior, it is still valid.

Jun 10 2020, 1:36 AM · Restricted Project

Jun 5 2020

ekatz added a comment to D81236: Improve LegacyPassManager API to correctly report modified status.

There seems to be a more profound bug underneath.
Is this how every pass that use LoopInfo handle it?
It seems like the real issue is with LoopInfo - an analysis shouldn't do a transformation.

Jun 5 2020, 3:16 AM · Restricted Project

Jun 1 2020

ekatz committed rG8a84158e5b96: [StructurizeCFG] Fix an incorrect comment, NFC. (authored by ekatz).
[StructurizeCFG] Fix an incorrect comment, NFC.
Jun 1 2020, 8:01 AM
ekatz added inline comments to D79037: [StructurizeCFG] Fix region nodes ordering.
Jun 1 2020, 3:41 AM · Restricted Project
ekatz committed rG85c308804966: [StructurizeCFG] Fix region nodes ordering (authored by ekatz).
[StructurizeCFG] Fix region nodes ordering
Jun 1 2020, 3:10 AM
ekatz closed D79037: [StructurizeCFG] Fix region nodes ordering.
Jun 1 2020, 3:09 AM · Restricted Project
ekatz added inline comments to D79037: [StructurizeCFG] Fix region nodes ordering.
Jun 1 2020, 1:01 AM · Restricted Project

May 30 2020

ekatz updated the diff for D79037: [StructurizeCFG] Fix region nodes ordering.

Update algorithm to use a custom GraphTraits and a NodeRef that holds the set of nodes for the subgraph.
This simplifies the algorithm quite a bit.

May 30 2020, 11:07 AM · Restricted Project

May 29 2020

ekatz added inline comments to D79037: [StructurizeCFG] Fix region nodes ordering.
May 29 2020, 11:23 PM · Restricted Project
ekatz committed rGf881c7967dbe: [tests] Fix AMDGPU test (authored by ekatz).
[tests] Fix AMDGPU test
May 29 2020, 12:35 PM
ekatz committed rGc710bb44a6b4: [Local] Prevent `invertCondition` from creating a redundant instruction (authored by ekatz).
[Local] Prevent `invertCondition` from creating a redundant instruction
May 29 2020, 11:28 AM
ekatz closed D80399: [Local] Prevent `invertCondition` from creating a redundant instruction.
May 29 2020, 11:27 AM · Restricted Project
ekatz committed rGdfc8244c2463: [PrintSCC] Fix printing a basic-block without a name (authored by ekatz).
[PrintSCC] Fix printing a basic-block without a name
May 29 2020, 10:23 AM
ekatz closed D80552: [PrintSCC] Fix printing a basic-block without a name.
May 29 2020, 10:23 AM · Restricted Project

May 28 2020

ekatz added inline comments to D80552: [PrintSCC] Fix printing a basic-block without a name.
May 28 2020, 8:51 PM · Restricted Project
ekatz added inline comments to D80552: [PrintSCC] Fix printing a basic-block without a name.
May 28 2020, 12:38 PM · Restricted Project
ekatz added inline comments to D79037: [StructurizeCFG] Fix region nodes ordering.
May 28 2020, 3:45 AM · Restricted Project

May 27 2020

ekatz updated the diff for D80552: [PrintSCC] Fix printing a basic-block without a name.

Added a test case.

May 27 2020, 1:36 PM · Restricted Project
ekatz added reviewers for D80552: [PrintSCC] Fix printing a basic-block without a name: foad, RKSimon, tejohnson, fhahn.
May 27 2020, 11:22 AM · Restricted Project
ekatz added a comment to D80399: [Local] Prevent `invertCondition` from creating a redundant instruction.

It looks like all those test diffs are caused by naming values. Can you add (perhaps even precommit) a test where your change makes a real improvement?

May 27 2020, 3:12 AM · Restricted Project

May 26 2020

ekatz added inline comments to D79037: [StructurizeCFG] Fix region nodes ordering.
May 26 2020, 1:37 PM · Restricted Project
ekatz updated the diff for D80399: [Local] Prevent `invertCondition` from creating a redundant instruction.

Updated tests to fit the change.

May 26 2020, 10:49 AM · Restricted Project
ekatz created D80552: [PrintSCC] Fix printing a basic-block without a name.
May 26 2020, 6:27 AM · Restricted Project
ekatz updated the diff for D79037: [StructurizeCFG] Fix region nodes ordering.

This is a second take of the solution, using SCCs instead of LoopInfo.
This algorithm can also handle irreducible loops, and, generally, any kind of cycle in the graph.

May 26 2020, 4:17 AM · Restricted Project

May 24 2020

ekatz added inline comments to D80399: [Local] Prevent `invertCondition` from creating a redundant instruction.
May 24 2020, 1:34 AM · Restricted Project

May 21 2020

ekatz created D80399: [Local] Prevent `invertCondition` from creating a redundant instruction.
May 21 2020, 12:27 PM · Restricted Project
ekatz added a comment to D80399: [Local] Prevent `invertCondition` from creating a redundant instruction.

Of course, this requires also fixing the tests affected by this change; but I wanted to present the change first, and if approved, I'll work on fixing the tests.

May 21 2020, 12:27 PM · Restricted Project
ekatz added a comment to D79037: [StructurizeCFG] Fix region nodes ordering.

TBH, I spent a small amount of time visualizing the new testcase, but couldn't figure out the problem, nor the solution. I see that the check for currentLoopSize is moved up the control flow a bit, but I didn't see why.

We need to move up the check for *CurrentLoopSize, as in the case it is zero, then we no longer need to skip nodes, and just continue to the next node/loop.
The simplified, new test case, demonstrate a case where it is needed.

May 21 2020, 11:21 AM · Restricted Project
ekatz updated the diff for D79037: [StructurizeCFG] Fix region nodes ordering.

Simplified the new test case.

May 21 2020, 11:21 AM · Restricted Project
ekatz committed rG111ddc57d380: [FlattenCFG] Fix `MergeIfRegion` in case then-path is empty (authored by ekatz).
[FlattenCFG] Fix `MergeIfRegion` in case then-path is empty
May 21 2020, 4:18 AM
ekatz closed D78881: [FlattenCFG] Fix `MergeIfRegion` in case then-path is empty.
May 21 2020, 4:18 AM · Restricted Project

May 20 2020

ekatz added a comment to D78881: [FlattenCFG] Fix `MergeIfRegion` in case then-path is empty.

Ping

May 20 2020, 10:54 AM · Restricted Project

May 14 2020

ekatz requested review of D79037: [StructurizeCFG] Fix region nodes ordering.
May 14 2020, 10:16 AM · Restricted Project
ekatz updated the diff for D79037: [StructurizeCFG] Fix region nodes ordering.

Fixed the bug causing an infinite loop in the new testcase.

May 14 2020, 10:16 AM · Restricted Project
ekatz committed rGc6c265527dd1: Revert "[StructurizeCFG] Fix region nodes ordering" (authored by ekatz).
Revert "[StructurizeCFG] Fix region nodes ordering"
May 14 2020, 8:06 AM
ekatz added a reverting change for rG897d8ee5cd69: [StructurizeCFG] Fix region nodes ordering: rGc6c265527dd1: Revert "[StructurizeCFG] Fix region nodes ordering".
May 14 2020, 8:06 AM
ekatz reopened D79037: [StructurizeCFG] Fix region nodes ordering.

Thanks both of you.
I'll revert and update the patch for it.

May 14 2020, 8:05 AM · Restricted Project
ekatz added a comment to D79037: [StructurizeCFG] Fix region nodes ordering.

Hi, This commit makes some compilations go into an infinite loop. I will try to prepare a reproducer.

Thanks.
Waiting for a test case...

May 14 2020, 5:52 AM · Restricted Project

May 13 2020

ekatz committed rG897d8ee5cd69: [StructurizeCFG] Fix region nodes ordering (authored by ekatz).
[StructurizeCFG] Fix region nodes ordering
May 13 2020, 5:54 AM
ekatz closed D79037: [StructurizeCFG] Fix region nodes ordering.
May 13 2020, 5:54 AM · Restricted Project

May 12 2020

ekatz updated the diff for D78881: [FlattenCFG] Fix `MergeIfRegion` in case then-path is empty.

Rename Invert2 variable to InvertCond2.
Hope it makes more sense.

May 12 2020, 3:43 AM · Restricted Project
ekatz added inline comments to D78881: [FlattenCFG] Fix `MergeIfRegion` in case then-path is empty.
May 12 2020, 12:30 AM · Restricted Project

May 11 2020

ekatz added a comment to D78881: [FlattenCFG] Fix `MergeIfRegion` in case then-path is empty.

Ping

May 11 2020, 1:02 AM · Restricted Project
ekatz added a comment to D79037: [StructurizeCFG] Fix region nodes ordering.

LGTM. It would be nice if you can move the following tests to the new file and demonstrate that that they are fixed. But that need not hold up this review any longer.

  • test/Transforms/StructurizeCFG/AMDGPU/backedge-id-bug.ll
  • test/Transforms/StructurizeCFG/AMDGPU/backedge-id-bug-xfail.ll

[EDIT] Actually added the test names.

May 11 2020, 1:02 AM · Restricted Project

May 10 2020

ekatz updated the diff for D79037: [StructurizeCFG] Fix region nodes ordering.

Added test for interleaved sibling loops, and change the test file name to "interleaved-loop-order.ll".

May 10 2020, 4:47 AM · Restricted Project

May 7 2020

ekatz added a comment to D79037: [StructurizeCFG] Fix region nodes ordering.

Actually, I take that back. Consider the following CFG:

A -> H1, H2
H1->B, C
B->H1
C->B, H2
H2->E
E->H2, F

A is the entry and F is the exit. The loops (H1, B, C) and (H2, E) are siblings with backedges B->H1 and E->H2 respectively. C is an exiting block in the first loop that branches to H2. One POT would be "B, F, E, H2, C, H1, A". When iterating in reverse, the loop with H1 is started first, but H2 from the second loop is encountered before the first loop is completed.

May 7 2020, 4:29 AM · Restricted Project

May 6 2020

ekatz added a comment to D79037: [StructurizeCFG] Fix region nodes ordering.

I see. It sure does look like PR25378. Then the only thing left is one new test with two sibling loops. The intention is to cover the case where CurrentLoop is not the parent of L.

I don't think it is possible, although I might be wrong.
Because the way I see it, for loop A be interfered with another loop B, while POT, there must be at least one block that belongs to loop A that is dependent on at least one block in loop B. But then loop B must be finished before loop A, and actually inside it. Otherwise, it won't be recognized as a separate loop.

May 6 2020, 9:40 AM · Restricted Project
ekatz added a comment to D79037: [StructurizeCFG] Fix region nodes ordering.

The problem with those tests is something else, and not produced by the wrong ordering. You may refer to PR25378 for an example.
Still, this fix the ordering issue and fixes PR41509 (and its duplicates).
I think we should insert this patch, and attend to the other bug as a separate issue.

May 6 2020, 6:57 AM · Restricted Project

May 4 2020

ekatz added a comment to D78881: [FlattenCFG] Fix `MergeIfRegion` in case then-path is empty.

Ping

May 4 2020, 10:35 PM · Restricted Project
ekatz updated the diff for D79037: [StructurizeCFG] Fix region nodes ordering.

Removed pushPathToAncestor.

May 4 2020, 6:22 AM · Restricted Project
ekatz added inline comments to D79037: [StructurizeCFG] Fix region nodes ordering.
May 4 2020, 6:22 AM · Restricted Project

May 3 2020

ekatz added inline comments to D79037: [StructurizeCFG] Fix region nodes ordering.
May 3 2020, 8:29 AM · Restricted Project
ekatz updated the diff for D79037: [StructurizeCFG] Fix region nodes ordering.

Done requested changes.

May 3 2020, 1:32 AM · Restricted Project
ekatz added inline comments to D79037: [StructurizeCFG] Fix region nodes ordering.
May 3 2020, 1:00 AM · Restricted Project

May 2 2020

ekatz added a comment to D79037: [StructurizeCFG] Fix region nodes ordering.

It seems the new commit is an incremental update on the previous commit. Can you please submit a single squashed commit, so that the change can be compared against the original state of the trunk?

I am not sure what do you mean. In this page in the Phabricator, under [Revision Contents]->[History] you can choose which of the patch revisions to diff. The default is "Base" against the latest "Diff no.".

May 2 2020, 5:47 AM · Restricted Project

May 1 2020

ekatz abandoned D78746: [LLVM-C] Match C-API integer types to the underlying used CPP-API.

Thanks for the references.
I'm abandoning this change, due to lack of interest.

May 1 2020, 10:08 PM · Restricted Project

Apr 30 2020

ekatz updated the diff for D79037: [StructurizeCFG] Fix region nodes ordering.

Done requested changes.

Apr 30 2020, 4:11 AM · Restricted Project
ekatz added inline comments to D79037: [StructurizeCFG] Fix region nodes ordering.
Apr 30 2020, 4:11 AM · Restricted Project

Apr 29 2020

ekatz added reviewers for D78881: [FlattenCFG] Fix `MergeIfRegion` in case then-path is empty: sameerds, cszide, zhendongsu.
Apr 29 2020, 11:58 PM · Restricted Project
ekatz added reviewers for D79037: [StructurizeCFG] Fix region nodes ordering: cszide, zhendongsu, bjope.
Apr 29 2020, 10:11 AM · Restricted Project

Apr 28 2020

ekatz created D79037: [StructurizeCFG] Fix region nodes ordering.
Apr 28 2020, 2:02 PM · Restricted Project
ekatz added a comment to D78746: [LLVM-C] Match C-API integer types to the underlying used CPP-API.

This is a fairly substantial ABI break not sure how to deal with this best.

Apr 28 2020, 5:51 AM · Restricted Project

Apr 26 2020

ekatz created D78881: [FlattenCFG] Fix `MergeIfRegion` in case then-path is empty.
Apr 26 2020, 10:05 AM · Restricted Project

Apr 25 2020

ekatz committed rG64249f177e46: [CodeExtractor] Fix extraction of a value used only by intrinsics outside of… (authored by ekatz).
[CodeExtractor] Fix extraction of a value used only by intrinsics outside of…
Apr 25 2020, 2:05 AM
ekatz closed D78749: [CodeExtractor] Fix extraction of a value used only by intrinsics outside of region.
Apr 25 2020, 2:05 AM · Restricted Project

Apr 24 2020

ekatz added inline comments to D78749: [CodeExtractor] Fix extraction of a value used only by intrinsics outside of region.
Apr 24 2020, 10:16 AM · Restricted Project
ekatz updated the diff for D78749: [CodeExtractor] Fix extraction of a value used only by intrinsics outside of region.

Update test to include debug info, and add some comments.

Apr 24 2020, 1:02 AM · Restricted Project
ekatz added inline comments to D78749: [CodeExtractor] Fix extraction of a value used only by intrinsics outside of region.
Apr 24 2020, 1:02 AM · Restricted Project

Apr 23 2020

ekatz created D78749: [CodeExtractor] Fix extraction of a value used only by intrinsics outside of region.
Apr 23 2020, 1:00 PM · Restricted Project
ekatz created D78746: [LLVM-C] Match C-API integer types to the underlying used CPP-API.
Apr 23 2020, 12:28 PM · Restricted Project

Apr 16 2020

ekatz committed rG03a9526fe5ad: [CGExprAgg] Fix infinite loop in `findPeephole` (authored by ekatz).
[CGExprAgg] Fix infinite loop in `findPeephole`
Apr 16 2020, 3:57 AM
ekatz closed D78098: [CGExprAgg] Fix infinite loop in `findPeephole`.
Apr 16 2020, 3:57 AM · Restricted Project

Apr 15 2020

ekatz updated the diff for D78098: [CGExprAgg] Fix infinite loop in `findPeephole`.

Changed the name of the test file to pr45476.cpp.

Apr 15 2020, 12:05 PM · Restricted Project
ekatz updated the diff for D78098: [CGExprAgg] Fix infinite loop in `findPeephole`.

Simplify function using IgnoreParenNoopCasts.

Apr 15 2020, 4:20 AM · Restricted Project
ekatz added inline comments to D78098: [CGExprAgg] Fix infinite loop in `findPeephole`.
Apr 15 2020, 4:20 AM · Restricted Project
ekatz added inline comments to D78098: [CGExprAgg] Fix infinite loop in `findPeephole`.
Apr 15 2020, 4:19 AM · Restricted Project

Apr 14 2020

ekatz added inline comments to D78098: [CGExprAgg] Fix infinite loop in `findPeephole`.
Apr 14 2020, 7:03 PM · Restricted Project
ekatz updated the diff for D78098: [CGExprAgg] Fix infinite loop in `findPeephole`.

Use the old structure of the algorithm, with the fix, as requested.

Apr 14 2020, 2:04 PM · Restricted Project
ekatz added inline comments to D78098: [CGExprAgg] Fix infinite loop in `findPeephole`.
Apr 14 2020, 2:04 PM · Restricted Project
ekatz updated the diff for D78098: [CGExprAgg] Fix infinite loop in `findPeephole`.

Added test.

Apr 14 2020, 5:49 AM · Restricted Project
ekatz created D78098: [CGExprAgg] Fix infinite loop in `findPeephole`.
Apr 14 2020, 4:45 AM · Restricted Project

Apr 12 2020

ekatz added a comment to D77963: Refactor StringMap.h, splitting StringMapEntry out to its own header..

The variables naming convention is inconsistent.
LLVM is using CamelCase while MLIR is using camelBack.
I know there have been some discussions regarding changing LLVM to use camelBack for variables as well, but I am not sure if there was any conclusion...

Apr 12 2020, 1:02 AM · Restricted Project

Apr 11 2020

ekatz committed rG3e8de2ed7449: [MLIR] Fix MLIR_MAIN_[SRC|INCLUDE]_DIR variables (authored by ekatz).
[MLIR] Fix MLIR_MAIN_[SRC|INCLUDE]_DIR variables
Apr 11 2020, 11:57 PM
ekatz closed D77943: [MLIR] Fix MLIR_MAIN_[SRC|INCLUDE]_DIR variables.
Apr 11 2020, 11:57 PM · Restricted Project
ekatz added a comment to D77943: [MLIR] Fix MLIR_MAIN_[SRC|INCLUDE]_DIR variables.

Note that MLIR_MAIN_INCLUDE_DIR is used in almost all (if not all) the CMakeLists.txt in the project.

Apr 11 2020, 8:31 AM · Restricted Project
ekatz created D77943: [MLIR] Fix MLIR_MAIN_[SRC|INCLUDE]_DIR variables.
Apr 11 2020, 7:59 AM · Restricted Project

Apr 3 2020

ekatz added a comment to D77144: [NFC] Remove waymarking because it improves performances.

"llvm/docs/ProgrammersManual.rst" needs to be updated as well.

Apr 3 2020, 9:38 PM · Restricted Project

Apr 1 2020

ekatz updated subscribers of D77144: [NFC] Remove waymarking because it improves performances.

Let me shed a little light on the Waymarking algorithm:

Apr 1 2020, 12:32 AM · Restricted Project

Mar 31 2020

ekatz committed rG154d517bc7d6: [ADT] Implement the Waymarking as an independent utility (authored by ekatz).
[ADT] Implement the Waymarking as an independent utility
Mar 31 2020, 7:12 AM

Mar 21 2020

ekatz committed rG34fd007aaf80: Revert "[ADT] Implement the Waymarking as an independent utility" (authored by ekatz).
Revert "[ADT] Implement the Waymarking as an independent utility"
Mar 21 2020, 1:52 PM
ekatz added a reverting change for rG73cf8abbe695: [ADT] Implement the Waymarking as an independent utility: rG34fd007aaf80: Revert "[ADT] Implement the Waymarking as an independent utility".
Mar 21 2020, 1:52 PM
ekatz committed rG73cf8abbe695: [ADT] Implement the Waymarking as an independent utility (authored by ekatz).
[ADT] Implement the Waymarking as an independent utility
Mar 21 2020, 5:53 AM