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

fix latest LLVM 11 compatibility issues #35329

Merged
merged 2 commits into from
Apr 22, 2020
Merged

Conversation

yhls
Copy link
Contributor

@yhls yhls commented Apr 1, 2020

  • CompositeType class has been removed
  • FileLineInfoSpecifier::Default has been renamed to RawValue

@vchuravy

@vchuravy vchuravy requested a review from vtjnash April 2, 2020 00:01
@yhls
Copy link
Contributor Author

yhls commented Apr 8, 2020

  • SequentialType class has been removed
  • CreateMemCpy now takes MaybeAligns instead of unsigneds

@yhls yhls force-pushed the yhls/llvm-11 branch 2 times, most recently from 8ce7fd8 to 4f8866e Compare April 8, 2020 03:59
Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

/Users/julia/buildbot/worker/package_macos64/build/src/llvm-late-gc-lowering.cpp:414:1: warning: control may reach end of non-void function [-Wreturn-type]
}
^
1 warning generated.
In file included from /Users/julia/buildbot/worker/package_macos64/build/src/codegen.cpp:853:
./cgutils.cpp:1349:28: warning: variable 'num_elements' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
            else if (auto *VT = dyn_cast<VectorType>(T))
                           ^~
./cgutils.cpp:1351:17: note: uninitialized use occurs here
            if (num_elements == 0)
                ^~~~~~~~~~~~
./cgutils.cpp:1349:18: note: remove the 'if' if its condition is always true
            else if (auto *VT = dyn_cast<VectorType>(T))
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./cgutils.cpp:1346:34: note: initialize the variable 'num_elements' to silence this warning
            uint64_t num_elements;
                                 ^
                                  = 0
1 warning generated.

Need to fix the generated warnings, but otherwise lgtm

src/llvm-late-gc-lowering.cpp Outdated Show resolved Hide resolved
@yhls yhls force-pushed the yhls/llvm-11 branch 2 times, most recently from 29232a0 to 5f08d9c Compare April 14, 2020 04:06
@yhls
Copy link
Contributor Author

yhls commented Apr 14, 2020

I've fixed the warnings, and addressed the following change in LLVM

  • deprecated overloads of IRBuilder::CreateCall and CallInst::Create have been removed

by making prepare_call_in in cgutils.cpp return a FunctionCallee instead of a Value*.

@yhls
Copy link
Contributor Author

yhls commented Apr 16, 2020

  • CallSite class has been removed

Copy link
Member

@vchuravy vchuravy left a comment

Choose a reason for hiding this comment

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

To use getNumElements, I'd have to explicitly cast T to StructType like in the following cases--do you suggest that I do this?

Probably want to use a dyn_cast then.

src/cgutils.cpp Show resolved Hide resolved
@yhls
Copy link
Contributor Author

yhls commented Apr 22, 2020

To use getNumElements, I'd have to explicitly cast T to StructType like in the following cases--do you suggest that I do this?

Probably want to use a dyn_cast then.

I meant to say that using getStructNumElements allows us to avoid having to cast explicitly, as opposed to using getNumElements with a dyn_cast, so is probably preferable. But I'm pretty indifferent about one over the other here.

@vchuravy
Copy link
Member

I meant to say that using getStructNumElements allows us to avoid having to cast explicitly, as opposed to using getNumElements with a dyn_cast, so is probably preferable. But I'm pretty indifferent about one over the other here.

It looks like LLVM is removing all the convenience getters, and it is cheaper to do the dyn_cast then to do an isa check and then do the cast inside getStructNumElements (which will do another isa check.)

yhls and others added 2 commits April 22, 2020 14:28
Changes in LLVM:
- `CompositeType` and `SequentialType` classes have been removed
- `FileLineInfoSpecifier::Default` has been renamed to `RawValue`
- `CreateMemCpy` now takes `MaybeAlign`s instead of `unsigned`s
- deprecated overloads of `IRBuilder::CreateCall` and `CallInst::Create` have
  been removed
- `CallSite` class has been removed
@yhls
Copy link
Contributor Author

yhls commented Apr 22, 2020

Ah, ok, changed to use getNumElements.

@vchuravy vchuravy merged commit 098ef24 into JuliaLang:master Apr 22, 2020
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.

3 participants