Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0005346OCamlOCaml generalpublic2011-08-21 09:202014-07-16 20:50
Reportermeurer 
Assigned To 
PrioritynormalSeveritytweakReproducibilityN/A
StatusfeedbackResolutionopen 
PlatformOSOS Version
Product Version3.12.1 
Target Versionafter-4.02.0Fixed 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.
Tagspatch
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
(0006330)
protz (manager)
2011-12-16 18:02

Hi,

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.

Cheers,

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

Not urgent enough for 4.01.

- 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 => after-4.02.0


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker