Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#11076 closed bug (fixed)

Demand information of foreign calls is wrong

Reported by: luite Owned by:
Priority: normal Milestone: 7.10.3
Component: Compiler Version: 7.10.2
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D1464
Wiki Page:

Description

Foreign calls of all types (including prim calls) use mkFCallId to create a fresh Id for each one. This Id claims to be strict in all arguments, which is not correct for foreign calls that allow lifted types to be passed. This also causes some trouble for GHCJS.

I hope to get a fix in 7.10.3. Can we leave the strictness sig out entirely perhaps? Most arguments would be trivially strict since they're unboxed.

Attachments (3)

forDmd_prim.cmm (96 bytes) - added by luite 4 years ago.
forDmd.hs (207 bytes) - added by luite 4 years ago.
ForDmdA.hs (516 bytes) - added by luite 4 years ago.

Download all attachments as: .zip

Change History (15)

Changed 4 years ago by luite

Attachment: forDmd_prim.cmm added

Changed 4 years ago by luite

Attachment: forDmd.hs added

Changed 4 years ago by luite

Attachment: ForDmdA.hs added

comment:1 Changed 4 years ago by luite

comment:2 Changed 4 years ago by luite

Milestone: 7.10.3

comment:3 Changed 4 years ago by luite

Changing the argument demand type from evalDmd to topDmd (in MkId.mkFCallId) fixes the problem for this test case and for GHCJS, and other than a minor (and I think harmless) change in one of the stranal test cases doesn't cause any validate issues.

I just want to make sure, is this a sane thing to do?

comment:4 Changed 4 years ago by simonpj

Luite, yes, I think that seems sane. Where precisely do you make this change?

Please add a clear Note to explain, referencing the ticket and giving an example.

It's late in the day for 7.10.3, but I'll leave that to Ben and Austin.

Simon

comment:5 Changed 4 years ago by luite

Differential Rev(s): Phab:D1407

comment:6 Changed 4 years ago by luite

Differential Rev(s): Phab:D1407Phab:D1464

comment:7 Changed 4 years ago by luite

See diff. I'll update the expected test results once my validate run finishes.

Should I also add the attached test case to the testsuite?

If it's too late for 7.10.3 then feel free to remove the milestone and just merge in HEAD, I'll try to find a workaround then.

comment:8 Changed 4 years ago by bgamari

I think we can swing 7.10.3. Can you add your testcase to the patch, however?

comment:9 Changed 4 years ago by luite

I've added the test case from this ticket. Does the change in strictness signature for T8598 look ok?

comment:10 Changed 4 years ago by Ben Gamari <ben@…>

In e090f1b/ghc:

Change demand information for foreign calls

Foreign calls may not be strict for lifted arguments. Fixes Trac #11076.

Test Plan: ./validate

Reviewers: simonpj, bgamari, austin

Subscribers: thomie

Differential Revision: https://phabricator.haskell.org/D1464

GHC Trac Issues: #11076

comment:11 Changed 4 years ago by bgamari

Resolution: fixed
Status: newclosed

comment:12 Changed 3 years ago by Ben Gamari <ben@…>

In 494907f/ghc:

testsuite: Add test for #11076
Note: See TracTickets for help on using tickets.