Page MenuHomePhabricator

bmahjour (Bardia)
User

Projects

User does not belong to any projects.

User Details

User Since
May 3 2019, 8:38 AM (15 w, 4 d)

Recent Activity

Wed, Aug 14

bmahjour added a comment to D65350: [DDG] Data Dependence Graph Basics.

A friendly reminder to kindly review and provide comments/approval. Thank you!

Wed, Aug 14, 10:43 AM · Restricted Project

Wed, Jul 31

bmahjour updated the diff for D65350: [DDG] Data Dependence Graph Basics.

Addressed comments from Michael and Min-Yih.

Wed, Jul 31, 12:28 PM · Restricted Project
bmahjour added a comment to D65350: [DDG] Data Dependence Graph Basics.

Addressed comments from Michael and Min-Yih.

Wed, Jul 31, 12:26 PM · Restricted Project

Fri, Jul 26

bmahjour created D65350: [DDG] Data Dependence Graph Basics.
Fri, Jul 26, 2:41 PM · Restricted Project

Jul 15 2019

bmahjour updated the diff for D64088: [DDG] DirectedGraph as a base class for various dependence graphs such as DDG and PDG..

Address comments on the unit test.

Jul 15 2019, 7:33 AM · Restricted Project

Jul 12 2019

bmahjour added inline comments to D64088: [DDG] DirectedGraph as a base class for various dependence graphs such as DDG and PDG..
Jul 12 2019, 10:27 AM · Restricted Project
bmahjour added a comment to D64088: [DDG] DirectedGraph as a base class for various dependence graphs such as DDG and PDG..

To not to be stuck in details and not block the dependence graph patches, we maybe should land the patch and work on it in-tree when its users materialize?

Sounds good, but I think to land it, it would be good if we have some unit tests for the current implementation, to make sure it builds, we don’t regress and have sanitizer coverage.

Jul 12 2019, 10:24 AM · Restricted Project
bmahjour updated the diff for D64088: [DDG] DirectedGraph as a base class for various dependence graphs such as DDG and PDG..

Address more review comments.

Jul 12 2019, 10:23 AM · Restricted Project

Jul 10 2019

bmahjour updated the diff for D64088: [DDG] DirectedGraph as a base class for various dependence graphs such as DDG and PDG..

Address second round of comments from Michael and Florian.

Jul 10 2019, 2:12 PM · Restricted Project
bmahjour added a comment to D64088: [DDG] DirectedGraph as a base class for various dependence graphs such as DDG and PDG..

To reduce the number of allocations, have you thought about making EdgeList contain the edges objects instead of pointers? The edges would have to be copied/moved into the list and edges could not be compared by identity. Is this semantic needed/are edge objects large?

Since the edge class does not contain the source node, the same edge object could be put into multiple outgoing edges lists. Is this supported?

I think we would be better off using pointers in this case, because of the following reasons:

  1. Using pointers gives the clients freedom to use polymorphic behavior.

I don't see why you would go the way implementing compile-time template polymorphism, but introduce vtables in derived classes.

I actually just realized that we cannot have a container of objects due to the CRTP idiom, because the edge type is not a complete type yet. If we don't use static polymorphism, then we need to use dynamic polymorphism and that requires a container of pointers. Either way we need to store pointers, it seems.

Jul 10 2019, 12:21 PM · Restricted Project

Jul 9 2019

bmahjour added a comment to D64088: [DDG] DirectedGraph as a base class for various dependence graphs such as DDG and PDG..

To reduce the number of allocations, have you thought about making EdgeList contain the edges objects instead of pointers? The edges would have to be copied/moved into the list and edges could not be compared by identity. Is this semantic needed/are edge objects large?

Since the edge class does not contain the source node, the same edge object could be put into multiple outgoing edges lists. Is this supported?

Jul 9 2019, 2:14 PM · Restricted Project
bmahjour added inline comments to D64088: [DDG] DirectedGraph as a base class for various dependence graphs such as DDG and PDG..
Jul 9 2019, 2:11 PM · Restricted Project
bmahjour updated the diff for D64088: [DDG] DirectedGraph as a base class for various dependence graphs such as DDG and PDG..

Address Michael's review comments.

Jul 9 2019, 1:48 PM · Restricted Project

Jul 4 2019

bmahjour added a comment to D64088: [DDG] DirectedGraph as a base class for various dependence graphs such as DDG and PDG..

Tests missing.

Jul 4 2019, 1:15 PM · Restricted Project
bmahjour added a comment to D64088: [DDG] DirectedGraph as a base class for various dependence graphs such as DDG and PDG..

Is there any plan on supporting GraphTraits in this patch?

Jul 4 2019, 1:08 PM · Restricted Project
bmahjour updated the summary of D64088: [DDG] DirectedGraph as a base class for various dependence graphs such as DDG and PDG..
Jul 4 2019, 6:56 AM · Restricted Project

Jul 2 2019

bmahjour created D64088: [DDG] DirectedGraph as a base class for various dependence graphs such as DDG and PDG..
Jul 2 2019, 11:01 AM · Restricted Project

Jun 4 2019

bmahjour added a reviewer for D62610: [DA] Add an option to control delinearization validity checks: fhahn.
Jun 4 2019, 2:06 PM · Restricted Project
bmahjour updated the diff for D62610: [DA] Add an option to control delinearization validity checks.

formatting change to reduce the number of new lines in the options description.

Jun 4 2019, 12:59 PM · Restricted Project
bmahjour updated the diff for D62610: [DA] Add an option to control delinearization validity checks.

Renamed the option to -da-disable-delinearization-checks and set the default value to "false". Updated LIT tests accordingly.

Jun 4 2019, 12:47 PM · Restricted Project

Jun 3 2019

bmahjour added a comment to D62610: [DA] Add an option to control delinearization validity checks.

I would have liked to invert the option though. Most similar options are set up such that they allow to opt-out of the sound behavior.

Jun 3 2019, 9:15 AM · Restricted Project

May 30 2019

bmahjour added a comment to D62610: [DA] Add an option to control delinearization validity checks.

As a debugging tool, this sounds fine. Not so sure about the tests, but I guess they are not hurting anyone :) And show good room for improvement.

Do you have any ideas how we might get the llvm form of them we have here (not the c form) to give good DA results without the miscompiles that the new option would produce? I was wondering if it was worth trying to get DA to return a dependency that is conditional on a set of predicates. Clients (we only have 3 at the moment) could treat predicated dependencies as "confused", or version the loop based on them.

May 30 2019, 2:42 PM · Restricted Project

May 29 2019

bmahjour created D62610: [DA] Add an option to control delinearization validity checks.
May 29 2019, 10:08 AM · Restricted Project