Skip to content

Force argument and result in register for imul with a constant#329

Merged
gretay-js merged 4 commits intooxcaml:mainfrom
gretay-js:fix_imul_reload
Oct 22, 2021
Merged

Force argument and result in register for imul with a constant#329
gretay-js merged 4 commits intooxcaml:mainfrom
gretay-js:fix_imul_reload

Conversation

@gretay-js
Copy link
Copy Markdown
Contributor

@gretay-js gretay-js commented Oct 12, 2021 •
edited
Loading

In reload, add a missing case for Iintop_imm Imul .
Without it, the compiler emits imulq $12, 72(%rsp) which is illegal because it has stack as destination instead of a register.

For example:

let foo v1 v2 v3 v4 v5 v6 v7 v8 v9 v10 v11 v12 v13 v14 v15 v16 v17 v18 =
     ( (8 * v1))
    +  ( (4 * v2))
    + ( (6 * v3))
    + ( (6 * v4))
    + (  (3 * v5))
    + ( (6 * v6))
    + (  (6 * v7))
    +  ( (6 * v8))
    +  ( (8 * v9))
    + (8 * v10)
    + (4 * v11)
    + (6 * v12)
    + (6 * v13)
    + (3 * v14)
    + (6 * v15)
    + (6 * v16)
    + (12 * v17)
    + (8 * v18)

let boo () =
  let v1 = Sys.opaque_identity 0 in
  let v2 = Sys.opaque_identity 0 in
  let v3 = Sys.opaque_identity 0 in
  let v4 = Sys.opaque_identity 0 in
  let v5 = Sys.opaque_identity 0 in
  let v6 = Sys.opaque_identity 0 in
  let v7 = Sys.opaque_identity 0 in
  let v8 = Sys.opaque_identity 0 in
  let v9 = Sys.opaque_identity 0 in
  let v10 = Sys.opaque_identity 0 in
  let v11 = Sys.opaque_identity 0 in
  let v12 = Sys.opaque_identity 0 in
  let v13 = Sys.opaque_identity 0 in
  let v14 = Sys.opaque_identity 0 in
  let v15 = Sys.opaque_identity 0 in
  let v16 = Sys.opaque_identity 0 in
  let v17 = Sys.opaque_identity 0 in
  let v18 = Sys.opaque_identity 0 in
  foo v1 v2 v3 v4 v5 v6 v7 v8 v9 v10 v11 v12 v13 v14 v15 v16 v17 v18

let () =
  ignore (Sys.opaque_identity (boo ()) : int)

causes assembler error

boo.s: Assembler messages:
boo.s:71: Error: operand size mismatch for `imul'
File "boo.ml", line 1:
Error: Assembler error, input left in file boo.s

@gretay-js
Force argument and result in register for imul with a constant
148e2c2 
In reload, missing case for (Iintop_imm ImulL) causes assembler error
@gretay-js gretay-js requested a review from xclerc as a code owner October 12, 2021 14:57
@mshinwell mshinwell added the bug Something isn't working label Oct 22, 2021
xclerc
xclerc approved these changes Oct 22, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@xclerc xclerc left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record, this is ocaml/ocaml#10628;
I think we should keep this information.

xclerc
xclerc reviewed Oct 22, 2021
View reviewed changes
@gretay-js gretay-js enabled auto-merge (squash) October 22, 2021 09:23
@gretay-js gretay-js merged commit daab68d into oxcaml:main Oct 22, 2021
mshinwell pushed a commit that referenced this pull request Oct 29, 2021
@gretay-js @mshinwell
Force argument and result in register for imul with a constant (#329)
db586c6 
* Force argument and result in register for imul with a constant

In reload, missing case for (Iintop_imm ImulL) causes assembler error

Port of ocaml/ocaml#10628
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xclerc xclerc xclerc approved these changes

Assignees

No one assigned

Labels

backend bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gretay-js @xclerc @mshinwell