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.

Sign up for GitHub

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

Jump to bottom

Support LLVM16 #3649

Closed
wants to merge 5 commits into from
Closed

Support LLVM16 #3649

wants to merge 5 commits into from

Conversation

QuLogic
Copy link
Contributor

@QuLogic QuLogic commented Apr 12, 2023

There was surprisingly little to do, but I was testing with the latest release, and not dev yet. I also wasn't sure where to add this to CI since you've dropped a lot of them, so it's only put in CircleCI for now.

Naturally, this depends on tinygo-org/go-llvm#45.

I'm not entirely sure the elaborated typedef change is correct, especially because the only failure I have is a few repeats of:

# _/builddir/build/BUILD/tinygo-0.27.0/_build/src/github.com/tinygo-org/tinygo/testdata/cgo
testdata/cgo/main.go:146:48: cannot use (*[7][2]C.int)(nil) (value of type *[7][2]C.int) as [4][7][2]C.int value in argument to C.arraydecay

but we'll see how it goes in CI for dev.

@QuLogic QuLogic added the enhancement New feature or request label Apr 12, 2023
@QuLogic QuLogic force-pushed the llvm16 branch 3 times, most recently from 6aaf776 to 7f420c4 Compare April 13, 2023 09:53
@QuLogic
Copy link
Contributor Author

QuLogic commented Apr 13, 2023

I realized there was another case for handling typedefs in makeDecayingASTType, so I also handled elaboraed typedefs there, but I was extermely conservative and only unwrapped if the elaborated type was a typedef.

Additionally, it appears that RISCV targets now have a CPU; I've added that in, but without any version checks, so that will probably break something.

@QuLogic
Copy link
Contributor Author

QuLogic commented Apr 15, 2023 •
edited
Loading

This now passes CI for LLVM16, but because the Circle CI part does not run go test, it's actually missing some issues:

  • In cgo tests, the converted structs now get a translated version of the C name, instead of a generically counted name, i.e., C._Ctype_struct___0 -> C.struct_point_t. Perhaps related to the elaborated types change?
  • In TestCompiler/channel.go, the argmemonly attribute is now memory(argmem: readwrite), but at the end of the list instead. This is the first note in the LLVM IR release notes
  • In interp tests, the pointers seem to have been simplified to ptr; it appears that opaque pointers are on by default now, and is probably similar to Update transform tests with opaque pointers #3566.

@QuLogic QuLogic force-pushed the llvm16 branch 2 times, most recently from dcee614 to a180e2b Compare April 16, 2023 01:41
@QuLogic
Copy link
Contributor Author

QuLogic commented Apr 16, 2023

This now works, AFAICT, at least against 0.27.0 and LLVM 16. I'm not sure which job to modify here to get full LLVM16 testing.

@QuLogic QuLogic marked this pull request as ready for review April 16, 2023 01:49
@aykevl
Copy link
Member

aykevl commented Apr 16, 2023

We don't really have full LLVM 16 testing until Espressif releases their rebased LLVM fork.
Testing in CircleCI seems good enough to me for now.

I will look at the changes later.

@QuLogic
Copy link
Contributor Author

QuLogic commented Apr 16, 2023

Now that I've succeeded at building 0.27.0, I've run against dev as well, and found one new file that needed changes, so here's a small update to fix that.

@QuLogic QuLogic force-pushed the llvm16 branch 2 times, most recently from a42849e to f98f38d Compare April 17, 2023 08:08
@aykevl
Copy link
Member

aykevl commented May 5, 2023

I'm looking into this now, in particular I'm working on full support for LLVM 16 (including static linking with LLVM and fixing the CGo issue).

@QuLogic
Copy link
Contributor Author

QuLogic commented May 6, 2023

Rebased with the merged go-llvm commit.

@deadprogram deadprogram added this to the v0.29.0 milestone May 18, 2023
aykevl added a commit that referenced this pull request May 22, 2023
@aykevl
all: switch to LLVM 16
f691827 
This commit adds support for LLVM 16 and switches to it by default. That
means three LLVM versions are supported at the same time: LLVM 14, 15,
and 16.

Part of this work was based on a PR by QuLogic:
#3649
But I also had parts of this already implemented in an old branch I
already made for LLVM 16.

The difference is that this commit is more complete:
  * It switches to LLVM 16 by default.
  * It updates some things to also make it work with a self-built LLVM.
  * It fixes the CGo bug in a slightly different way, and also fixes
    another one not included in the original PR.
  * It does not keep compiler tests passing on older LLVM versions. I
    have found this to be quite burdensome and therefore don't generally
    do this - the smoke tests should hopefully catch most regressions.
aykevl added a commit that referenced this pull request May 22, 2023
@aykevl
all: switch to LLVM 16
4ee0a86 
This commit adds support for LLVM 16 and switches to it by default. That
means three LLVM versions are supported at the same time: LLVM 14, 15,
and 16.

Part of this work was based on a PR by QuLogic:
#3649
But I also had parts of this already implemented in an old branch I
already made for LLVM 16.

The difference is that this commit is more complete:
  * It switches to LLVM 16 by default.
  * It updates some things to also make it work with a self-built LLVM.
  * It fixes the CGo bug in a slightly different way, and also fixes
    another one not included in the original PR.
  * It does not keep compiler tests passing on older LLVM versions. I
    have found this to be quite burdensome and therefore don't generally
    do this - the smoke tests should hopefully catch most regressions.
@aykevl aykevl mentioned this pull request May 22, 2023
Merged
@aykevl
Copy link
Member

aykevl commented May 22, 2023

Hi @QuLogic I have made a LLVM 16 PR here: #3741
I did it in a separate PR because I prefer switching to a new LLVM version entirely (not off-by-default) to catch more bugs, and because I needed to fix that CGo bug anyway. I hope you don't mind!
One difference that may be important to you is that I don't run all the compiler tests for older LLVM versions: there are frequent IR changes and supporting multiple LLVM versions is just too much hassle for little gain.

aykevl added a commit that referenced this pull request Jul 6, 2023
@aykevl
all: switch to LLVM 16
c951a21 
This commit adds support for LLVM 16 and switches to it by default. That
means three LLVM versions are supported at the same time: LLVM 14, 15,
and 16.

Part of this work was based on a PR by QuLogic:
#3649
But I also had parts of this already implemented in an old branch I
already made for LLVM 16.

The difference is that this commit is more complete:
  * It switches to LLVM 16 by default.
  * It updates some things to also make it work with a self-built LLVM.
  * It fixes the CGo bug in a slightly different way, and also fixes
    another one not included in the original PR.
  * It does not keep compiler tests passing on older LLVM versions. I
    have found this to be quite burdensome and therefore don't generally
    do this - the smoke tests should hopefully catch most regressions.
@deadprogram deadprogram modified the milestones: v0.29.0, v0.30.0 Jul 20, 2023
aykevl added a commit that referenced this pull request Aug 21, 2023
@aykevl
all: switch to LLVM 16
c731470 
This commit adds support for LLVM 16 and switches to it by default. That
means three LLVM versions are supported at the same time: LLVM 14, 15,
and 16.

Part of this work was based on a PR by QuLogic:
#3649
But I also had parts of this already implemented in an old branch I
already made for LLVM 16.

The difference is that this commit is more complete:
  * It switches to LLVM 16 by default.
  * It updates some things to also make it work with a self-built LLVM.
  * It fixes the CGo bug in a slightly different way, and also fixes
    another one not included in the original PR.
  * It does not keep compiler tests passing on older LLVM versions. I
    have found this to be quite burdensome and therefore don't generally
    do this - the smoke tests should hopefully catch most regressions.
aykevl added a commit that referenced this pull request Sep 18, 2023
@aykevl
all: switch to LLVM 16
e9c814b 
This commit adds support for LLVM 16 and switches to it by default. That
means three LLVM versions are supported at the same time: LLVM 14, 15,
and 16.

Part of this work was based on a PR by QuLogic:
#3649
But I also had parts of this already implemented in an old branch I
already made for LLVM 16.

The difference is that this commit is more complete:
  * It switches to LLVM 16 by default.
  * It updates some things to also make it work with a self-built LLVM.
  * It fixes the CGo bug in a slightly different way, and also fixes
    another one not included in the original PR.
  * It does not keep compiler tests passing on older LLVM versions. I
    have found this to be quite burdensome and therefore don't generally
    do this - the smoke tests should hopefully catch most regressions.
aykevl added a commit that referenced this pull request Sep 18, 2023
@aykevl
all: switch to LLVM 16
7ed80f5 
This commit adds support for LLVM 16 and switches to it by default. That
means three LLVM versions are supported at the same time: LLVM 14, 15,
and 16.

This commit includes work by QuLogic:

  * Part of this work was based on a PR by QuLogic:
    #3649
    But I also had parts of this already implemented in an old branch I
    already made for LLVM 16.
  * QuLogic also provided a CGo fix here, which is also incorporated in
    this commit:
    #3869

The difference with the original PR by QuLogic is that this commit is
more complete:
  * It switches to LLVM 16 by default.
  * It updates some things to also make it work with a self-built LLVM.
  * It fixes the CGo bug in a slightly different way, and also fixes
    another one not included in the original PR.
  * It does not keep compiler tests passing on older LLVM versions. I
    have found this to be quite burdensome and therefore don't generally
    do this - the smoke tests should hopefully catch most regressions.
aykevl added a commit that referenced this pull request Sep 18, 2023
@aykevl
all: switch to LLVM 16
4c1cc09 
This commit adds support for LLVM 16 and switches to it by default. That
means three LLVM versions are supported at the same time: LLVM 14, 15,
and 16.

This commit includes work by QuLogic:

  * Part of this work was based on a PR by QuLogic:
    #3649
    But I also had parts of this already implemented in an old branch I
    already made for LLVM 16.
  * QuLogic also provided a CGo fix here, which is also incorporated in
    this commit:
    #3869

The difference with the original PR by QuLogic is that this commit is
more complete:
  * It switches to LLVM 16 by default.
  * It updates some things to also make it work with a self-built LLVM.
  * It fixes the CGo bug in a slightly different way, and also fixes
    another one not included in the original PR.
  * It does not keep compiler tests passing on older LLVM versions. I
    have found this to be quite burdensome and therefore don't generally
    do this - the smoke tests should hopefully catch most regressions.
aykevl added a commit that referenced this pull request Sep 18, 2023
@aykevl
all: switch to LLVM 16
d5bd41a 
This commit adds support for LLVM 16 and switches to it by default. That
means three LLVM versions are supported at the same time: LLVM 14, 15,
and 16.

This commit includes work by QuLogic:

  * Part of this work was based on a PR by QuLogic:
    #3649
    But I also had parts of this already implemented in an old branch I
    already made for LLVM 16.
  * QuLogic also provided a CGo fix here, which is also incorporated in
    this commit:
    #3869

The difference with the original PR by QuLogic is that this commit is
more complete:
  * It switches to LLVM 16 by default.
  * It updates some things to also make it work with a self-built LLVM.
  * It fixes the CGo bug in a slightly different way, and also fixes
    another one not included in the original PR.
  * It does not keep compiler tests passing on older LLVM versions. I
    have found this to be quite burdensome and therefore don't generally
    do this - the smoke tests should hopefully catch most regressions.
@deadprogram
Copy link
Member

@QuLogic this PR is also included in #3741 thank you for your efforts! Now closing.

@QuLogic QuLogic deleted the llvm16 branch September 18, 2023 18:23
deadprogram pushed a commit that referenced this pull request Sep 18, 2023
@aykevl @deadprogram
all: switch to LLVM 16
1d7543e 
This commit adds support for LLVM 16 and switches to it by default. That
means three LLVM versions are supported at the same time: LLVM 14, 15,
and 16.

This commit includes work by QuLogic:

  * Part of this work was based on a PR by QuLogic:
    #3649
    But I also had parts of this already implemented in an old branch I
    already made for LLVM 16.
  * QuLogic also provided a CGo fix here, which is also incorporated in
    this commit:
    #3869

The difference with the original PR by QuLogic is that this commit is
more complete:
  * It switches to LLVM 16 by default.
  * It updates some things to also make it work with a self-built LLVM.
  * It fixes the CGo bug in a slightly different way, and also fixes
    another one not included in the original PR.
  * It does not keep compiler tests passing on older LLVM versions. I
    have found this to be quite burdensome and therefore don't generally
    do this - the smoke tests should hopefully catch most regressions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
v0.30.0
Development

Successfully merging this pull request may close these issues.
3 participants
@QuLogic @aykevl @deadprogram