Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0005346OCamlOCaml generalpublic2011-08-21 09:202015-11-27 18:25
Assigned To 
PlatformOSOS Version
Product Version3.12.1 
Target VersionlaterFixed in Version 
Summary0005346: Improve Cmm code generator for "for" loops.
DescriptionThe code generated by cmmgen for Ufor loops doesn't optimize in case of constant low and high expressions. This is fixed by a special case where the initial comparison can be skipped in case of constant low and high subexpressions.

In addition, this patch also reorders the generated code by moving the initial comparison out of the catch block in the general case. This may enable certain optimizations in later phases.
Attached Filespatch file icon 0001-Improve-Cmm-code-generator-for-for-loops.patch [^] (3,146 bytes) 2011-08-21 09:20 [Show Content]

- Relationships

-  Notes
protz (manager)
2011-12-16 18:02


First of all, thanks for the patch. Disclaimer: I'm in no way an expert of Cmm. Here are some preliminary remarks, though.
- Your patch adds complexity for what seem to be little benefit. The test is an initial one, it's not in the loop. You're just skipping a couple instructions. Is it worth the extra complexity of your approach? Can you justify better why we should make that case harder to understand in cmmgen?
- There are no comments in your patch, which resonates with the point above. Moreover, the current version has an id_prev identifier defined. While I fail to see why it's there in the first place (your version of the test seems both shorter and correct), I'd appreciate if you could share your thoughts on this.

Thinking about performance, even in the case of nested loops, say

for i = 0 to n; do
  for j = 0 to i; do

then the extra test that might have been run n+1 times wouldn't be skipped since j is not a constant expression. This seems to me like a very ad-hoc optimization that adds extra complexity for close to zero benefit. I'd like to be proved wrong, though :) so please let me know why you think this optimization has to be included.

A benchmark on some real-world code may help me see better why you want this.


xleroy (administrator)
2013-07-19 11:43

Not urgent enough for 4.01.
xleroy (administrator)
2015-11-23 15:47

No activity on this PR in the last 4 years. Should we keep it alive or suspend it?
shinwell (developer)
2015-11-23 18:10

We have some vague plans for local CPS constructs in flambda in the future; I wouldn't be surprised, having expressed these looping constructions like that, if things such as this PR just fall out in the wash.

So I would vote for closing this.

- Issue History
Date Modified Username Field Change
2011-08-21 09:20 meurer New Issue
2011-08-21 09:20 meurer File Added: 0001-Improve-Cmm-code-generator-for-for-loops.patch
2011-12-16 18:02 protz Note Added: 0006330
2011-12-21 14:23 gasche Status new => feedback
2012-09-06 16:43 doligez Target Version => 4.00.1+dev
2012-09-20 16:27 doligez Target Version 4.00.1+dev => 4.01.0+dev
2013-07-19 11:43 xleroy Note Added: 0009812
2013-07-19 11:43 xleroy Target Version 4.01.0+dev => 4.01.1+dev
2013-10-09 14:15 doligez Tag Attached: patch
2014-05-25 20:20 doligez Target Version 4.01.1+dev => 4.02.0+dev
2014-07-16 20:50 doligez Target Version 4.02.0+dev => 4.02.1+dev
2014-09-04 00:25 doligez Target Version 4.02.1+dev => undecided
2014-09-22 22:13 doligez Target Version undecided => 4.02.2+dev / +rc1
2015-01-15 00:32 doligez Target Version 4.02.2+dev / +rc1 => 4.03.0+dev
2015-11-23 15:47 xleroy Note Added: 0014803
2015-11-23 18:10 shinwell Note Added: 0014810
2015-11-27 18:25 frisch Target Version 4.03.0+dev => later

Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker