Re: Add relations support to OpenScop library.

[Tobias Grosser]

On 08/29/2010 01:39 PM, Riyadh Baghdadi wrote:
> Hi,
>
> - The patch is based on :
> https://alchemy.futurs.inria.fr/svn/users/bastoul/clan/branches/openscop
> revision 33668,

Hi Riyadh,

great to see so much work done on this. I just tried to apply the patch,
but it fails at several places.

Are you sure you used the most recent svn version for this work? If I go
back to revision

https://alchemy.futurs.inria.fr/svn/users/bastoul/scoplib/branches/openscop@31235

almost everything except Makefile.am applies cleanly.

Furthermore, as future openscop development will happen on the git
repository it is probably easier for all of us, if your patch is based
on the latest code in the git repository at:

http://repo.or.cz/w/openscop.git

The code in the repository is the same as the last svn commit on the
inria repository.

Cheers
Tobi

[Riyadh Baghdadi]

Thank you tobias :)
As soon as I get feedback from cedric about code, I'll switch to Git and send you back the code,

2010/8/30 Tobias Grosser <gro...@fim.uni-passau.de>

--
Riyadh

[Tobias Grosser]

On 08/30/2010 08:08 PM, Riyadh Baghdadi wrote:
> Thank you tobias :)
> As soon as I get feedback from cedric about code, I'll switch to Git and
> send you back the code,

Hi Riyadh,

this is fine. Switching to git is not that urgent. However there is
still the problem, that your patch is not using the most recent svn
revision. If I use svn revision 33668 (the one you said the patch should
apply), I get a lot of failures.

Here one of the failing parts of your patch:

######################################################################
Index: source/statement.c
===================================================================
--- source/statement.c (revision 11)
+++ source/statement.c (working copy)
@@ -85,10 +85,11 @@
* \param level Number of spaces before printing, for each line.
**
* - 30/04/2008: first version.
+ * - 28/08/2010: adapted to use openscop_statement_matrix_p.
*/
void
-openscop_statement_print_structure(FILE * file, openscop_statement_p
statement,
- int level)
+openscop_statement_matrix_print_structure(FILE * file,
openscop_statement_matrix_p statement,
+ int level)
######################################################################

You are referencing "* - 30/04/2008: first version."

This line was removed by Cedric in this commit:
-------------
Author: bastoul <bastoul@d2347644-e8e6-0310-9099-9a715ae17836>
Date: Tue May 11 16:23:11 2010 +0000

remove matrix list

git-svn-id:
https://alchemy.futurs.inria.fr/svn/users/bastoul/scoplib/branches/openscop@31246
-------------

At least the changes to the file source/statement.c are based on an old
version of openscop. As Cedric removed all history annotations in the
comments, I believe adding a new one
"+ * - 28/08/2010: adapted to use openscop_statement_matrix_p."
is probably a step backwards.

Do you think you could create an updated patch even before Cedric starts
reviewing the changes? I would also like to look at your patch, but
unfortunately there have been quite some changes in openscop, which I
have the feeling did not make it in your svn checkout. An updated patch
that takes those changes into account is a lot easier to review for me.

Thanks a lot
Tobi

[Riyadh Baghdadi]

2010/8/30 Tobias Grosser <gro...@fim.uni-passau.de>

Are you sure you used the most recent svn version for this work? If I go back to revision

https://alchemy.futurs.inria.fr/svn/users/bastoul/scoplib/branches/openscop@31235

almost everything except Makefile.am applies cleanly.

This is the updated patch,

Here is once again a summary on the patch :

This patch for OpenScop Library enables the current implementation to read/write/manipulate relation based scops,
- It adds relation support for OpenScop lib.
- Most of the old matrix based functions have been renamed to make a clear distinction between the matrix and the relation based implmentations.
- You find attached a test file that you can include with the library, the test file is also a hello world example for any one who wants to see quickly how he can use OpenScop in his project.
- The patch was tested against scop files generated by graphite, and it works correctly, (there is still one missing patch to make every thing clean from the side of graphite, I'll send it soon to the graphite amiling list).
- I switched to the latest svn repo : https://alchemy.futurs.inria.fr/svn/users/bastoul/scoplib/branches/openscop@https:34355
- Cedric: the names I modified the names of variables in the relation_structure to what you suggested,

Unfortunately I want be very reactive during the next 5 months, Cedric wants to complete to work on my current version, I hope that he finds my work useful.

Thanks,

--
Riyadh