Closed cocreature closed 8 years ago
Thank you for your contribution! I have not yet had time to completely review your PR, but I took a brief look over it and everything I saw looked good.
Do you have a sense of what fraction of the language-reference and API level changes necessary to catch up with 3.8 you've covered?
Take your time with reviewing, there are obvious problems (such as things I commented out for now) and I’ll hopefully get around to fixing at least some of them.
I’m not really sure what fraction I’ve covered. I’ve mostly gone through and tried to find out how things need to be changed to compile and then started running the tests. In particular that means that I didn’t cover any additions (e.g. OrcJIT) apart from some additional attributes.
The two biggest parts that are still missing are adapting to the new metadata api and the new personality/catchpad stuff. Then we should at least cover the things that were possible before but adapted to the new API. There are other changes I still need to do (see the test failures and the things commented out), but nothing that looked very complicated.
I think it would be better to add the new features in a separate PR since this one is already getting fairly large.
Short status update:
The metadata stuff turns out to be quite tricky. Metadata is now a separate hierarchy but it can be embedded using MetadataAsValue
and ValueAsMetadata
in both directions. It requires changes to Operand
and is therefore quite invasive (also Operand
has no correspondence in llvm, so I’m unsure about the best representation). To make matters worse, the C api only gives access to metadata embedded in values, the go bindings have basically duplicated the C api for that reason. Overall I don’t feel super confident in that area so if somebody else wants to pick up and add the metadata stuff, I’d be more than happy. If not I’ll try to figure it out, but it’s probably going to take quite some time.
Also it might make sense to merge it (into a branch) before that is finished, as that makes it easier for others to participate in the process.
You sound like I felt when I started looking at metadata for llvm-3.6.
I should be able to take a look, and at least merge your work onto an llvm-3.8 branch in my repo, tomorrow. Thanks for the continued effort!
I managed to mostly get metadata to work. The local metadata test still fails, I haven’t managed to figure out how that is represented in the human readable assembly. I assume it’s still possible since there is LocalAsMetadata
. The other tests work, so hopefully I didn’t screw up too badly.
I'm getting some link errors. Are you building with --enable-shared or -fshared-llvm ?
It appears one of my problems is that the llvm build has turned of rtti. The other is a missing include required in ValuesC.cpp.
I haven’t used --enable-shared and I tried building with -fshared-llvm
and without -fshared-llvm
and it builds successfully either way. Sadly I can’t find the cmake command I used for my build, but I remember having to enable rtti. A missing include sounds weird, what error are you seeing exactly?
Just a warning:
.../llvm-general/llvm-general/out/llvm-3.8.0/install/include/llvm/IR/Type.h:350:16: warning: inline function ‘llvm::Type* llvm::Type::getSequentialElementType() const’ used but never defined [enabled by default] inline Type *getSequentialElementType() const;
shows up because that function is declared inline in Type.h (included in ValueC.cpp), but only defined in DerivedTypes.h. Adding the include of DerivedTypes.h clears up the warning.
I've tried improving the logic in Setup.hs to extract flags from llvm-config, so rtti gets turned on or off as appropriate.
Okay, my initial merge of this PR is now in this repo as the llvm-3.8 branch. It's been rebased off of the llvm-3.5 branch to pick up some small advances that had been made there after you branched off.
I have not yet merged in your metadata work.
Closing, see #168 for the remaining changes.
I’m opening this PR now even though it’s not finished to avoid duplicated work. The current state is that it compiles and 176 tests are passing. I plan to keep working on the things that are not yet working, but if anybody wants to pick it up before I finish it or help me do that (I’m not sure how much time I’ll have available), go for it.