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

excessive inlining of jump tables #7449

Closed
vicuna opened this issue Jan 3, 2017 · 5 comments · Fixed by #10458
Closed

excessive inlining of jump tables #7449

vicuna opened this issue Jan 3, 2017 · 5 comments · Fixed by #10458

Comments

@vicuna
Copy link

vicuna commented Jan 3, 2017

Original bug ID: 7449
Reporter: markghayden
Assigned to: @chambart
Status: assigned (set by @chambart on 2017-01-03T20:59:28Z)
Resolution: open
Priority: normal
Severity: minor
Platform: AMD64
OS: MacOS
OS Version: 10.12.2
Version: 4.04.0
Category: middle end (typedtree to clambda)
Tags: flambda
Monitored by: @gasche @ygrek

Bug description

See the attached code. The [to_string] function is inlined at all call sites, even with no optimization specified at the command line (!?!). Note that each call site includes a full copy of the jump table and all the jump branches. The test case here has 30 items. I've seen this with larger cases (300 items). Maybe I'm missing something, but it seems this is really off here since this behavior can result in significant code growth. A work-around is to annotate with '[@inline never]'.

Steps to reproduce

Compile with 4.04.0 or 4.05.0.dev trunk. Note that for 4.05.0.dev, the lookup table is used if all the return values are constants and perhaps that is OK to inline. Added a [failwith] for one of the cases forces the use of jump tables, at least on AMD64.

ocamlopt -c -S du.ml

Additional information

See attached ML file. Assembler output will follow.

File attachments

@vicuna
Copy link
Author

vicuna commented Jan 3, 2017

Comment author: @alainfrisch

Is this this flambda? I cannot reproduce in non-flambda mode, except by passing a large -inline command-line argument to ocamlopt.

@vicuna
Copy link
Author

vicuna commented Jan 3, 2017

Comment author: markghayden

All my testing was with flambda-enabled compilers.

@github-actions
Copy link

github-actions bot commented May 9, 2020

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label May 9, 2020
@lthls
Copy link
Contributor

lthls commented May 9, 2020

I can confirm that this bug is still present; the size estimation for switches doesn't seem to take the size of the actions into account.
Cc @chambart

@github-actions github-actions bot removed the Stale label Jun 10, 2020
@github-actions
Copy link

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

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

Successfully merging a pull request may close this issue.

4 participants