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

Duplicated expression when compiling with -g #6805

Closed
vicuna opened this issue Mar 8, 2015 · 2 comments

Comments

Projects
None yet
1 participant
@vicuna
Copy link

commented Mar 8, 2015

Original bug ID: 6805
Reporter: @chambart
Status: closed (set by @maranget on 2015-12-03T17:03:37Z)
Resolution: fixed
Priority: normal
Severity: major
Version: 4.02.1
Target version: 4.03.0+dev / +beta1
Fixed in version: 4.03.0+dev / +beta1
Category: back end (clambda to assembly)
Monitored by: @gasche

Bug description

When compiling this example extracted from frama-c, some expression
get duplicated, including variable declarations. Note that without -g
the generated output is correct.

This may no trigger any real bug with the current compiler, but the
absence of duplicated variables is something that I expect in my
flambda branch.

Steps to reproduce

compile with
ocamlc -c -g -dlambda example.ml

the outputs contains this:

(switch* switcher/1070
case int 0:
(let
(match/1039 =a (field 1 match/1030)
match/1040 =a (field 0 match/1039)
match/1041 =a (field 0 match/1040))
(catch
(if (!= match/1041 1)
(if (>= match/1041 3) (exit 2) (exit 6))
(exit 2))
with (6)
(let
(match/1055 =a (field 2 match/1030)
match/1056 =a (field 0 match/1055)
match/1057 =a (field 0 match/1056))
(catch
(if (!= match/1057 1)
(if (>= match/1057 3) (exit 2) (exit 7))
(exit 2))
with (7) (exit 1)))))
case int 1:
(let
(match/1039 =a (field 1 match/1030)
match/1040 =a (field 0 match/1039)
match/1041 =a (field 0 match/1040))
(catch
(if (!= match/1041 1)
(if (>= match/1041 3) (exit 2) (exit 6))
(exit 2))
with (6)
(let
(match/1055 =a (field 2 match/1030)
match/1056 =a (field 0 match/1055)
match/1057 =a (field 0 match/1056))
(catch
(if (!= match/1057 1)
(if (>= match/1057 3) (exit 2) (exit 7))
(exit 2))
with (7) (exit 1)))))

File attachments

@vicuna

This comment has been minimized.

Copy link
Author

commented Dec 2, 2015

Comment author: @alainfrisch

Bumping priority, as this sounds quite serious.

The bug is probably in switch.ml. It gets called from matching.ml to create a switch on integers 3, 5, 6, 7 and it produces a switch on arg-3 with cases 0, 1, 2, 3, 4 (instead of 0, 2, 3, 4).

I'm not familiar with this piece of code, but my analysis is the following:

  • When the set of cases is "dense" enough, it is compiled to a "integer switch".

  • In switch.ml, function make_switch, the array "tbl" is initialized with 0; this value will not be modified for "holes" in the set. This results in implicit sharing with the first case.

One could perhaps fix that by detecting in Switch.make_switch the presence of holes, and add one more "dummy" action for them. I've attached a tentative fix, but it would be good if someone more aware of this piece of code (Luc?) could look at it...

@vicuna

This comment has been minimized.

Copy link
Author

commented Dec 3, 2015

Comment author: @maranget

Thank you for reporting and for the analysis.

In the described situation: switch emitted, some holes (missing constructors),
and no failure, some action must be choosed for holes. The action used
is the first of switch. Hence this action has to be declared shared.

All this is rather complex and not that robust. At least the PR is fixed.

@vicuna vicuna closed this Dec 3, 2015

@vicuna vicuna added the back-end label Mar 14, 2019

@vicuna vicuna added this to the 4.03.0 milestone Mar 14, 2019

@vicuna vicuna added the bug label Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.