Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize bounds checking #5345

Closed
vicuna opened this issue Aug 20, 2011 · 3 comments
Closed

Optimize bounds checking #5345

vicuna opened this issue Aug 20, 2011 · 3 comments
Labels

Comments

@vicuna
Copy link

vicuna commented Aug 20, 2011

Original bug ID: 5345
Reporter: meurer
Status: closed (set by @xavierleroy on 2012-09-25T18:07:21Z)
Resolution: fixed
Priority: normal
Severity: tweak
Version: 3.12.1
Category: ~DO NOT USE (was: OCaml general)
Related to: #5360
Monitored by: meyer bitbckt @protz mehdi @ygrek @hcarty pascal_cuoq @alainfrisch

Bug description

Here is a sequence of patches to improve the code selection for Ccheckbound. The first patch avoids the separate right shifting for block headers in most cases. The second and third patches further improve the code selection for amd64 and i386. Ccheckbound is now a sequence of cmp,jbe instead of mov,shr,cmp,jbe for the common memory/immediate case.

File attachments

@vicuna
Copy link
Author

vicuna commented Dec 21, 2011

Comment author: @protz

Hi,

Can you provide benchmarks for real-world situations where this is useful? Also, there's a compile flag that allows one to disable array bounds checking (-unsafe). Can you provide a rationale for optimizing this? I would tend to think that people who want performance disable array bounds checking.

Thanks,

jonathan

@vicuna
Copy link
Author

vicuna commented Dec 21, 2011

Comment author: @hcarty

A speed increase with bounds checking enabled would certainly be welcome. I would rather not give up the safety of bounds checking to gain speed when possible.

This assumes that the complexity of the patch is acceptable to the OCaml maintainers and that benchmarks show this patch provides a speed increase.

@vicuna
Copy link
Author

vicuna commented Dec 22, 2011

Comment author: @xavierleroy

Applied patch 0001 (the processor-independent part) in SVN trunk, commit 11934.

Even though array indexing with constant indices isn't that common, the patch is short, sweet, clever, and applies to all targets, so, let's go!

I prefer not to add x86-specific operations for this purpose, though. The "op mem, cst" form used to be a win over "mov reg, mem; op reg, cst" for early processors like the Pentium, but I doubt it makes much of a difference with contemporary x86 processors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant