[Monetdb-developers] [PATCH] Loop Lifted Pathfinder sources
Hi,
Please find attached a (compressed) patch which loop lifts all loops in the pathfinder sources such that variable declarations go at the top of the scope the loop is in, or a higher scope if more appropriate.
Rationale: I don't feel like appending stdc99 flags in a (possible) Gentoo ebuild for the next release.
In other words, this patch removes the C99 requirement, which I think more people will benefit from, such that MonetDB/* cleanly compiles with --disable-strict.
Please consider applying this patch to the current CVS sources.
My apologies for the large attachment on this list; there are simply lots of loops to "lift".
dank fabian.
Fabian Groffen wrote:
Hi,
Please find attached a (compressed) patch which loop lifts all loops in the pathfinder sources such that variable declarations go at the top of the scope the loop is in, or a higher scope if more appropriate.
Rationale: I don't feel like appending stdc99 flags in a (possible) Gentoo ebuild for the next release.
In other words, this patch removes the C99 requirement, which I think more people will benefit from, such that MonetDB/* cleanly compiles with --disable-strict.
Please consider applying this patch to the current CVS sources.
My apologies for the large attachment on this list; there are simply lots of loops to "lift".
Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&da...
Monetdb-developers mailing list Monetdb-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/monetdb-developers
On Thu, Nov 02, 2006 at 07:59:50AM +0100, Martin Kersten wrote:
dank fabian.
Fabian Groffen wrote:
Hi,
Please find attached a (compressed) patch which loop lifts all loops in the pathfinder sources such that variable declarations go at the top of the scope the loop is in, or a higher scope if more appropriate.
Rationale: I don't feel like appending stdc99 flags in a (possible) Gentoo ebuild for the next release.
In other words, this patch removes the C99 requirement, which I think more people will benefit from, such that MonetDB/* cleanly compiles with --disable-strict.
Just for the records: there is more C99 specific (i.e., beyond C89) code in Pathfinder than for-loop-inlined variable declarations; gcc w/o --stdc99 might only complain about the for-loop-inlined variable declarations; other compiles might/will complain about more (macro's with variable argument lists, "//" comments, name enum initializations ...); hence, even with/after this patch, Pathfinder code still is/requires C99.
Stefan
Please consider applying this patch to the current CVS sources.
My apologies for the large attachment on this list; there are simply lots of loops to "lift".
Stefan,
Thanks for the addition. It has always been our objective to compile on many platforms easily, even if this requires a slight adjustment of our personal coding styles. (// comments, declaration order, indenting,...)
So I sincerely hope (expect ;-))that someone from the pathfinder team takes over the step taken by Fabian and remove the C99 specifics. It is a one-day job with great benefit for the community.
I am looking forward to the next check-in.
Stefan Manegold wrote:
On Thu, Nov 02, 2006 at 07:59:50AM +0100, Martin Kersten wrote:
dank fabian.
Fabian Groffen wrote:
Hi,
Please find attached a (compressed) patch which loop lifts all loops in the pathfinder sources such that variable declarations go at the top of the scope the loop is in, or a higher scope if more appropriate.
Rationale: I don't feel like appending stdc99 flags in a (possible) Gentoo ebuild for the next release.
In other words, this patch removes the C99 requirement, which I think more people will benefit from, such that MonetDB/* cleanly compiles with --disable-strict.
Just for the records: there is more C99 specific (i.e., beyond C89) code in Pathfinder than for-loop-inlined variable declarations; gcc w/o --stdc99 might only complain about the for-loop-inlined variable declarations; other compiles might/will complain about more (macro's with variable argument lists, "//" comments, name enum initializations ...); hence, even with/after this patch, Pathfinder code still is/requires C99.
Stefan
Please consider applying this patch to the current CVS sources.
My apologies for the large attachment on this list; there are simply lots of loops to "lift".
On Wed, Nov 01, 2006 at 10:05:44PM +0100, Fabian Groffen wrote:
Hi,
Please find attached a (compressed) patch which loop lifts all loops in the pathfinder sources such that variable declarations go at the top of the scope the loop is in, or a higher scope if more appropriate.
Rationale: I don't feel like appending stdc99 flags in a (possible) Gentoo ebuild for the next release.
In other words, this patch removes the C99 requirement, which I think more people will benefit from, such that MonetDB/* cleanly compiles with --disable-strict.
Hi all,
though I acknowledge the amount of work Fabian has put into this, I don't really see the sense behind this patch.
The Pathfinder code heavily depends on many many C99'isms, and we all agree that the use of C99 features makes the code much more readable and---most importantly---less prone to errors.
The patch that Fabian distributed here only removes a tiny part of the C99 dependency, namely variable declarations within `for (...)' clauses. I think this specific part of C99 dependence has never been a concern in our project. These are rather straightforward to remove (as Fabian did), but also never really made any trouble during compilation.
What makes our code really C99 dependent are mainly variable initializers in our code. We have plenty of situations where we do
type varname[] = { [pos1] = value1, [pos2] = value2, ... };
or
struct foo = (foo) { .bar = v1, .baz = v2 };
etc.
*These* are the situations where we need the C99 dependency. And these are also the situations where we *don't* want to rewrite our code to ANSI C. The availability of these language features are quite crucial to the style we program in Pathfinder. Removing them would make our code *very* fragile, as we would suddenly depend on the *order* in which, e.g., bindings occur in an array.
I don't have any real problems with the patch Fabian provided. It is a matter of taste where one prefers to declare variables. The path does *not*, however, remove our dependency on C99 features.
@Fabian: What is the reason that you don't want to have the -std=c99 flag in your next ebuild? The compilers available for Linux provide excellent C99 support. Only the ignorance of Microsoft in their compiler suite makes the use of C99 appear a bad thing.
Regards,
Jens
P.S.: I'm not yet through with a successful native compile of Pathfinder on Windows using the Intel compiler. It seems like also the Intel compiler has some problems with variable initializers, but only if they get a bit more complex (we had this on the Linux side some time ago, icc v7 or so). This seems to only affect two or three situations in our code, and we were already discussing solutions to this here in Garching.
On 02-11-2006 08:40:00 +0100, Jens Teubner wrote:
What makes our code really C99 dependent are mainly variable initializers in our code. We have plenty of situations where we do
Ok, too bad my compiler didn't complain about that.
@Fabian: What is the reason that you don't want to have the -std=c99 flag in your next ebuild? The compilers available for Linux provide excellent C99 support. Only the ignorance of Microsoft in their compiler suite makes the use of C99 appear a bad thing.
Well, MonetDB at the moment is an unmaintainable beast within Gentoo that I'm completely fed up with. So, unless MonetDB will behave normally, like an autotooled package should, I'll yank it from the tree as it costs me way too much time to maintain it. Another option for me is to just yank XQuery support, but in that case I prefer removing MonetDB as a whole. Currently the MonetDB package in Gentoo is hard masked -- and you know what that means.
You can see the initial patch as a signal that I want to help out doing the dirty work. If, however, you don't feel like investing in it, neither will I.
On Thu, Nov 02, 2006 at 09:05:20AM +0100, Fabian Groffen wrote:
@Fabian: What is the reason that you don't want to have the -std=c99 flag in your next ebuild? The compilers available for Linux provide excellent C99 support. Only the ignorance of Microsoft in their compiler suite makes the use of C99 appear a bad thing.
Well, MonetDB at the moment is an unmaintainable beast within Gentoo that I'm completely fed up with. So, unless MonetDB will behave normally, like an autotooled package should, I'll yank it from the tree as it costs me way too much time to maintain it. Another option for me is to just yank XQuery support, but in that case I prefer removing MonetDB as a whole. Currently the MonetDB package in Gentoo is hard masked -- and you know what that means.
Hi Fabian,
I guess I cannot follow you here. I can only read this as an argument regarding the packaging of our software, where the core MonetDB and the Pathfinder package don't differ in any way. The Pathfinder package merely adds one compiler flag more, and neither GCC (the prime compiler I suppose on Gentoo) nor ICC (the only other reasonable compiler that I am aware of for Linux platforms) have any trouble with that.
You can see the initial patch as a signal that I want to help out doing the dirty work. If, however, you don't feel like investing in it, neither will I.
I do appreciate the work you are investing here. I just wanted to stress that this particular piece of work did not do any change to our C99 dependency.
Regards,
Jens
On Thu, Nov 02, 2006 at 09:54:15AM +0100, Jens Teubner wrote:
On Thu, Nov 02, 2006 at 09:05:20AM +0100, Fabian Groffen wrote:
@Fabian: What is the reason that you don't want to have the -std=c99 flag in your next ebuild? The compilers available for Linux provide excellent C99 support. Only the ignorance of Microsoft in their compiler suite makes the use of C99 appear a bad thing.
Well, MonetDB at the moment is an unmaintainable beast within Gentoo that I'm completely fed up with. So, unless MonetDB will behave normally, like an autotooled package should, I'll yank it from the tree as it costs me way too much time to maintain it. Another option for me is to just yank XQuery support, but in that case I prefer removing MonetDB as a whole. Currently the MonetDB package in Gentoo is hard masked -- and you know what that means.
Hi Fabian,
I guess I cannot follow you here. I can only read this as an argument regarding the packaging of our software, where the core MonetDB and the Pathfinder package don't differ in any way. The Pathfinder package merely adds one compiler flag more, and neither GCC (the prime compiler I suppose on Gentoo) nor ICC (the only other reasonable compiler that I am aware of for Linux platforms) have any trouble with that.
You can see the initial patch as a signal that I want to help out doing the dirty work. If, however, you don't feel like investing in it, neither will I.
I do appreciate the work you are investing here. I just wanted to stress that this particular piece of work did not do any change to our C99 dependency.
Regards,
Jens
We know (now) that this patch doesn't solve the C99 issues. But this patch makes it possible as fabian pointed out to compile all components of MonetDB without extra gcc options on Gentoo. This will make it much easier to maintain the MonetDB product family on Gentoo, which is currently the costing way to much time (without any payback (ie not used because it breaks way to many times)).
Therefor we still would like to integrate this patch. Any objections?
Niels
Hi Niels,
we are fine with the integration of this patch.
Cheers, --Teggy
On Nov 3, 2006, 10:24 AM, Niels Nes wrote with possible deletions:
[...] We know (now) that this patch doesn't solve the C99 issues. But this patch makes it possible as fabian pointed out to compile all components of MonetDB without extra gcc options on Gentoo. This will make it much easier to maintain the MonetDB product family on Gentoo, which is currently the costing way to much time (without any payback (ie not used because it breaks way to many times)).
Therefor we still would like to integrate this patch. Any objections?
Hi All,
I tried to apply Fabian's patches, but cannot check them in, yet.
First, I had to fix the patches, to get the code compiled with icc. Basically, icc (correctly) complained about code like
switch (n->kind) { unsigned int col = 0, row = 0;
case la_lit_tbl: ... }
with ======== /net/corona.ins.cwi.nl/export/scratch0/manegold/Monet/Testing/Current/pathfinder/compiler/algebra/prop/prop_const.c(305): error #185: dynamic initialization in unreachable code unsigned int col = 0, row = 0; ^
/net/corona.ins.cwi.nl/export/scratch0/manegold/Monet/Testing/Current/pathfinder/compiler/algebra/prop/prop_const.c(304): error #589: transfer of control bypasses initialization of: variable "col" (declared at line 305) variable "row" (declared at line 305) switch (n->kind) { ^
compilation aborted for /net/corona.ins.cwi.nl/export/scratch0/manegold/Monet/Testing/Current/pathfinder/compiler/algebra/prop/prop_const.c (code 2) make[8]: *** [libprop_la-prop_const.lo] Error 1 ========
Second, the patch seem to break the algebra implementation --- most algebra tests and some more test fail with the patch applied, while they don't without.
Hence, finally applying these patches will require some more time-consuming debugging --- but not tonight --- actually, I cannot tell, yet, when I'll have time for that ...
Stefan
On Fri, Nov 03, 2006 at 10:38:19AM +0100, Torsten Grust wrote:
Hi Niels,
we are fine with the integration of this patch.
Cheers, --Teggy
On Nov 3, 2006, 10:24 AM, Niels Nes wrote with possible deletions:
[...] We know (now) that this patch doesn't solve the C99 issues. But this patch makes it possible as fabian pointed out to compile all components of MonetDB without extra gcc options on Gentoo. This will make it much easier to maintain the MonetDB product family on Gentoo, which is currently the costing way to much time (without any payback (ie not used because it breaks way to many times)).
Therefor we still would like to integrate this patch. Any objections?
-- | Torsten Grust | torsten.grust@gmail.com
Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&da... _______________________________________________ Monetdb-developers mailing list Monetdb-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/monetdb-developers
On 03-11-2006 16:18:41 +0100, Stefan Manegold wrote:
Hence, finally applying these patches will require some more time-consuming debugging --- but not tonight --- actually, I cannot tell, yet, when I'll have time for that ...
Ohw. That's a pity. I ran mTest, but couldn't compare against testweb since that was completely blank.
I suggest you don't invest in this useless adventure, and do something useful with your time instead.
On Fri, Nov 03, 2006 at 04:24:19PM +0100, Fabian Groffen wrote:
On 03-11-2006 16:18:41 +0100, Stefan Manegold wrote:
Hence, finally applying these patches will require some more time-consuming debugging --- but not tonight --- actually, I cannot tell, yet, when I'll have time for that ...
Ohw. That's a pity. I ran mTest, but couldn't compare against testweb since that was completely blank.
wasn't me ;-)
I suggest you don't invest in this useless adventure, and do something useful with your time instead.
I will...
Stefan
Dear Jens,
Thank you for the contribution, because it sheds a better light on the rationale behind choices. It also shows the personal value attached to the issues using a strong tone of voice.
Finding a solution is a matter of analysis of the real problems and the effort required by someone to adjust the/his work, or to recognize that some results are not achievable.
I respect the coding style, but also would like to understand the ratio for them. So a complete list of C99 dependencies is as important as to see where/how this affect the config/compilation schemes.
Jens Teubner wrote:
The Pathfinder code heavily depends on many many C99'isms, and we all agree that the use of C99 features makes the code much more readable and---most importantly---less prone to errors.
The patch that Fabian distributed here only removes a tiny part of the C99 dependency, namely variable declarations within `for (...)' clauses. I think this specific part of C99 dependence has never been a concern in our project. These are rather straightforward to remove (as Fabian did), but also never really made any trouble during compilation.
What makes our code really C99 dependent are mainly variable initializers in our code. We have plenty of situations where we do
type varname[] = { [pos1] = value1, [pos2] = value2, ... };
This seems like a poor programming style to me, because it leaves elements un-initialized. A major source of errors. I tend to adhire to the scheme 'always initialize upon declaration and also finalization'. (e.g. in M5 I spent quite some effort to remove/reinitialize everything upon closing the system to ease detection of errors. In M4 we mark released memory for the same purpose).
or
struct foo = (foo) { .bar = v1, .baz = v2 };
etc.
*These* are the situations where we need the C99 dependency. And these are also the situations where we *don't* want to rewrite our code to ANSI C. The availability of these language features are quite crucial to the style we program in Pathfinder. Removing them would make our code *very* fragile, as we would suddenly depend on the *order* in
Please explain the fragility aspect to me. Also the order aspect is unclear. As far as I recall it is just a textual shorthand for a declaration directly followed by an initialization.
which, e.g., bindings occur in an array.
I don't have any real problems with the patch Fabian provided. It is a matter of taste where one prefers to declare variables. The path does *not*, however, remove our dependency on C99 features.
@Fabian: What is the reason that you don't want to have the -std=c99 flag in your next ebuild? The compilers available for Linux provide excellent C99 support. Only the ignorance of Microsoft in their compiler suite makes the use of C99 appear a bad thing.
The impact on portability should be documented. And perhaps a conclusion should be taken that MonetDB/* ceases to be supported on some. It is then a conscious decision on what community you want your products to be taken up.
Regards,
Jens
P.S.: I'm not yet through with a successful native compile of Pathfinder on Windows using the Intel compiler. It seems like also the Intel compiler has some problems with variable initializers, but only if they get a bit more complex (we had this on the Linux side some time ago, icc v7 or so). This seems to only affect two or three situations in our code, and we were already discussing solutions to this here in Garching.
participants (7)
-
Fabian Groffen -
Fabian Groffen -
Jens Teubner -
Martin Kersten -
Niels Nes -
Stefan Manegold -
Torsten Grust