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

LLVM 15 #4327

Merged
merged 22 commits into from
Apr 4, 2023
Merged

LLVM 15 #4327

merged 22 commits into from
Apr 4, 2023

Conversation

jemc
Copy link
Member

@jemc jemc commented Feb 28, 2023

@jemc jemc self-assigned this Feb 28, 2023
@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Feb 28, 2023
@jemc
Copy link
Member Author

jemc commented Mar 3, 2023

I can't figure out why the "full program" runner is exiting with status 1, without any output to explain why. It's not a crash that I could catch by trying to run the runner itself in lldb - it appears to be a clean exit without crash.

@kulibali - Can you take a look and see if you have any ideas?

@jemc jemc requested a review from chalcolith March 3, 2023 20:25
@chalcolith
Copy link
Member

When I debug into it on Windows, it calls _CliOptions.apply(), then CommandSpec.leaf(), then CommandSpec._assertName(). The second value returned from the call to nm.values() is \x0, although nm->_ptr is "runner". It then calls error, and exits with code 1.

@chalcolith
Copy link
Member

This program demonstrates the problem (and another one):

actor Main
  new create(env: Env) =>
    let str = "Hello, world."
    env.out.print(str)

    for ch in str.values() do
      env.out.print("ch is '" + USize.from[U8](ch).string() + "'")

      if ch == 0 then
        env.exitcode(1)
        break
      end
    end

For me it produces:

Hello, world.
ch is '
ch is '
ch is '

So the string concat is not working right either.

@jemc
Copy link
Member Author

jemc commented Mar 4, 2023

Thanks for narrowing down to a minimal program!

@SeanTAllen SeanTAllen removed the discuss during sync Should be discussed during an upcoming sync label Mar 7, 2023
jemc added 9 commits March 7, 2023 19:32
This optimization pass is not safe, and so it had to be removed.

This pass tries to remove calls to `pony_realloc` by making the original
pointer bigger to match the later realloc size, but it doesn't do any analysis
to prove that the reallocation didn't have some semantic importance.
For example, when using `Array.chop` or `String.chop` followed by a `push`
onto the left side of the chop point, the left side must be reallocated
in order to avoid mutating the right side. But this pass doesn't do any
analysis to check if the pointer may possibly be accessible from some SSA,
so it doesn't know that removing the reallocation will break the semantics.
@jemc
Copy link
Member Author

jemc commented Mar 11, 2023

There's one more error left in CI that I think could be resolved by disabling zstd in LLVM (since we're not using it anyway).

@kulibali - do you know what the right variable is to disable that?

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Mar 11, 2023
@chalcolith
Copy link
Member

@jemc I believe it's LLVM_ENABLE_ZSTD

@jemc jemc added the do not merge This PR should not be merged at this time label Mar 13, 2023
@jemc
Copy link
Member Author

jemc commented Mar 13, 2023

This is ready for review, but I don't want to merge it until addressing the open question I posed above.

@jemc jemc marked this pull request as ready for review March 13, 2023 22:30
@jemc jemc changed the title WIP: LLVM 15 LLVM 15 Mar 14, 2023
@jemc jemc requested review from chalcolith and SeanTAllen and removed request for chalcolith March 15, 2023 03:48
@jemc jemc added changelog - changed Automatically add "Changed" CHANGELOG entry on merge and removed do not merge This PR should not be merged at this time labels Mar 15, 2023
@SeanTAllen
Copy link
Member

@kulibali can you give this another review so we can merge it in?

@chalcolith
Copy link
Member

Ok, I'm busy this weekend but I'll try to take a look on Monday.

@SeanTAllen SeanTAllen merged commit 6806b6b into ponylang:main Apr 4, 2023
@ponylang-main ponylang-main removed the discuss during sync Should be discussed during an upcoming sync label Apr 4, 2023
github-actions bot pushed a commit that referenced this pull request Apr 4, 2023
github-actions bot pushed a commit that referenced this pull request Apr 4, 2023
@jemc jemc deleted the llvm15-jemc branch April 4, 2023 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - changed Automatically add "Changed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants