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

Get rid of obsolete __ArrayEq lowering & revise core.internal.array.equality #3456

Closed
wants to merge 3 commits into from

Conversation

kinke
Copy link
Member

@kinke kinke commented Jun 2, 2020

No description provided.

@kinke
Copy link
Member Author

kinke commented Jun 3, 2020

Pinging @JohanEngelen. I gotta go to sleep now. ;)

@kinke kinke force-pushed the arrayeq branch 3 times, most recently from 404651c to ca6de40 Compare June 19, 2020 20:55
@kinke kinke changed the title Get rid of obsolete __ArrayEq lowering Get rid of obsolete __ArrayEq lowering & revise core.internal.array.equality Jun 19, 2020
@kinke kinke marked this pull request as ready for review June 19, 2020 23:53
@kinke
Copy link
Member Author

kinke commented Jun 19, 2020

Geez, that was quite a bumpy road. I'll try to upstream this.

@kinke
Copy link
Member Author

kinke commented Jun 20, 2020

The ultimate goal would be to get rid of the backend-specific array comparison code emission (incl. identical use-memcmp logic) by always lowering to __equals, but I haven't been able to untangle this mess sufficiently yet.

kinke added 3 commits June 20, 2020 05:48
…plates

Debugging failing fail_compilation/ice19762.d (stack overflow in LDC)
was a trip down the rabbit hole.

By the looks of it, the root of all evil is again the frontend not
properly validating IndexExpressions - this time, for structs with fwd-
referencing errors (analogous to the problem for opaque structs with
compilable/ice20044.d, see https://issues.dlang.org/show_bug.cgi?id=20959).

The stack overflow happens in LLVM when trying to create a GEP for
core.internal.array.equality.at(), when checking the struct element type
(with cycles, i.e., containing a field of itself) for sizedness. This
little function now uses `pragma(inline, true)`, so it was newly
codegen'd although needsCodegen() returns false.

With the new available_externally emission into each referencing CU, I
think this enforced regular emission from the module members tree
shouldn't be required anymore, allowing to work around the test
regression and possibly more issues with speculative instantiations.

After adapting the codegen/inlining_templates.d lit-test accordingly (no
IR definitions anymore in .ll file, because available_externally doesn't
make it there), I've noticed that a lambda in imported and inlined
call_enforce_with_default_template_params() wasn't emitted - it got
culled by alreadyOrWillBeDefined().
Function/delegate literals aren't culled anymore.
@kinke
Copy link
Member Author

kinke commented Sep 29, 2020

The frontend and druntime parts have been upstreamed and are available in 2.094: dlang/dmd#11212 and dlang/druntime#3142

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

Successfully merging this pull request may close these issues.

1 participant