LLVM is currently in the process of migrating towards opaque pointers. Opaque Pointers — LLVM 16.0.0git documentation has some documentation for this migration, but the TL;DR is that pointers like i32* are being replaced with an opaque pointer type ptr. You can test opaque pointers today by passing -opaque-pointers to opt or -mllvm -opaque-pointers to clang.
We have recently reached a milestone where it is possible to compile most C/C++ code, with or without optimization, in opaque pointer mode. We pass the non-bitcode portions of llvm-test-suite, and it’s possible to bootstrap clang (at least in some configurations).
If you maintain an LLVM-based compiler frontend, an out-of-tree backend, or a fork of LLVM, then now would be a very good time to test whether your code is compatible with opaque pointers. The document linked above contains migration instructions that may be helpful.
If you are interested in helping out with the remaining opaque pointer migration tasks, git grep getPointerElementType and git grep Address::deprecated should point out all remaining places where we rely on pointer element types. Help would be particularly appreciated on the clang side, as well as other non-LLVM projects like MLIR or Polly.
Let me know if you have any questions or concerns.
I wanted to follow up about the textual representation of address spaces, since that came up in the last RFC. What is the new syntax, and are we happy with it? In any case, the syntax should be added to the linked opaque pointer documentation.
Lastly, thanks to everyone working on this project! Arthur shared these preliminary compile time results with our team (maybe you collected them), and they are pretty good. I summarize that as a rough 2% compile time improvement in optimized build configurations (thinlto, O3, etc). We expected to see some compile time benefits, but we can finally actually measure them now.
The current syntax is ptr for address space zero and ptr addrspace(N) for address space N. I believe in previous discussions using something like p0 and pN was suggested as a more compact syntax. ptr and ptr(N) would be another choice. The nice thing about ptr addrspace(N) is that it matches the syntax we use in many other places to specify address spaces. On the other hand, it may be more verbose than necessary. I don’t have a strong opinion on the syntax choice here.
Yeah, initial compile-time results look good. I expect we’ll see more wins once we don’t have to support both modes anymore. Something to keep in mind though is that enabling opaque pointers also has a non-trivial effect on codegen (there are differences in code size), so we might be comparing apples and oranges here We do try to generate the same code for typed and opaque pointers, but GEPs (with their essentially arbitrary “source element type”) make that especially hard. And well, part of the point of opaque pointers is that they enable more optimization…
With af6b9939aac065ce19bdaaf9fc54a8368367f33f check-llvm/clang/lld now pass with a bootstrapped build using -O3 + opaque pointers, at least on my version of Linux.
Have a couple of questions (since this change is gonna result in a boatload of work for our out-of-tree custom aliasing infrastructure!):
Is there an ETA for opaque pointers being the default?
Do we think any optimizations will begin to convert typed pointers to opaque pointers?
Is there an ETA for typed pointers to be removed?
A technical query too - for our Burst HPC# compiler we have custom aliasing that basically works by having metadata that stores additional information about structs and how they can/cannot alias with their contents. We rely on being able to walk the typed pointers to other structs to work out the aliasing. With opaque pointers we are gonna have to sideband this information somehow (I guess we’ll just have to keep a full metadata for each struct, remembering the real type of the opaque pointers). Just wondered if anyone had any better suggestions on the best way to keep this kind of information?
There isn’t a firm timeline yet, I expect that it will be at least another month or two before we can even consider a switch. The critical path is done now, but there is a long tail of issues that need to be resolved first.
Opaque pointers are a separate mode. Within one LLVMContext, you can either have all typed pointers or all opaque pointers, they are never mixed. So no, if you start out with typed pointers, those will stay typed pointers (as long as they are supported at all).
This will likely happen shorty after the default has been switched, we’re confident that it will not be reverted, and all tests have been migrated to opaque pointers. The core problem is that it’s not really feasible for us to test both modes (because test output will differ for pretty much any test involving pointers, which is a large fraction of them), so we cannot retain typed pointer support for long.
Encoding this using metadata is generally the right approach (just like TBAA does), though it would be helpful to have some more information on your system works. I didn’t quite understand where you attach metadata (are you adding it to load/stores, saying that this access is to a certain field of a certain struct?) and how that would interact with pointer types.
There isn’t a firm timeline yet, I expect that it will be at least another month or two before we can even consider a switch. The critical path is done now, but there is a long tail of issues that need to be resolved first.
Cool - I think we’ll just assume this is in LLVM 15 and work from there.
Encoding this using metadata is generally the right approach (just like TBAA does), though it would be helpful to have some more information on your system works. I didn’t quite understand where you attach metadata (are you adding it to load/stores, saying that this access is to a certain field of a certain struct?) and how that would interact with pointer types.
So at present we store global metadata such that each named struct has a unique named metadata associated with it <struct-name><suffix>, and that metadata records whether fields of the struct have any additional no-alias guarantees. We then use the struct type + this metadata to be able to walk the typed pointers through any accesses to it (memory intrinsics, calls, load/stores), and work out whether we have better aliasing guarantees. So it seems like having additional per-named-struct metadata for the pointee types will handle our case.
We previously talked about making a best effort to leave typed pointer support in place without testing guarantees for some period (maybe one major release cycle). That is, we may convert the in tree tests to use opaque pointers, but we won’t intentionally remove any code needed for typed pointer support for some time and allow out-of-tree users to continue using it at their own risk. Does that sound reasonable?
Please can you add to the documentation details of how GEP will work with opaque pointers.
Maybe include examples of using the IR Builder API to create GEP’s with opaque pointers.
I think what we can do is to delay the actual removal of typed pointer support until after LLVM 15 has been branched off, so that the LLVM 15 release defaults to opaque pointers, but still has typed pointer support that was not given much chance to bit-rot yet. This gives people another 6 months to migrate, without increasing upstream maintenance burden unduly.
I don’t think it’s possible to justify retaining typed pointer support beyond that point, as the complexity associated with it is massive and pervasive, and we won’t be seeing the full benefits of opaque pointers until typed pointers are gone for good. It will also block further work that is based on opaque pointers, such as GEP canonicalization.
In order to support opaque pointers, GEPs have been changed to have an explicit “source element type”, which is no longer implied by the pointer type. However, this change has already happened many years ago. The API for creating a GEP is independent of whether opaque pointers are used or not. In fact, all (current) IRBuilder APIs don’t care whether you use opaque pointers or not, the calls are always the same.
I’ve got an out-of-tree front end that we try to keep in sync with trunk, not a release branch, so it’s not as much buffer as I was hoping for, but I guess it’s reasonable. I know this work has been in progress and well-publicized for quite a long time, and we’ve been trying to get things switched over. I think we’ll be able to manage with the plan you’ve proposed.
Hi,
How is this going to work with bitcode generated by earlier LLVM versions?
Concretely, I’m thinking about DXIL, which is LLVM bitcode generated by the DirectX DXC compiler. It uses LLVM 3.something and struct function arguments from HLSL appear as pointers in DXIL.
A compiler that takes DXIL needs to know the types behind these pointers. I assume there are more cases like that out there, that take old, non-opaque-pointer IR.
In the LangRef, I see there is an elementtype(<ty>) attribute for function parameters. In the review it was restricted to intrinsics only, but I guess that could be relaxed, so the bitcode reader adds attributes when reading IR with type pointers?
After ⚙ D120471 [Bitcode] Fully support opaque pointer auto upgrade lands, we will support upgrading old bitcode to work with opaque pointers. The auto-upgrade adds the necessary new IR elements (like load types or byval types) based on old pointer element types.
I don’t think we will relax this in-tree (the restriction is an important deterrent), but if that is appropriate for your LLVM fork, sure, you can allow elementtype on all functions and add the attribute for all functions during auto-upgrade. This should be fairly easy to do (there is already code handling elementtype auto-upgrade for certain intrinsics, you just need to do that unconditionally).
A likely better solution (in that it does not require patching the IR verifier) is to add metadata instead, see for example what AMDGPU does with kernel_arg_type etc metadata. That seems somewhat similar to your situation.
That’s nice, is there any way to access the pointer element types after the BitcodeReader?
Ideally it would just be some out-of-tree passes that work with upstream llvm
Anything that works with upstream llvm would be nice, really I guess adding metadata would need to be done in the (llvm 3.x) frontend which I have no control over? Or do you mean adding metadata in the BitcodeReader?
No, this information is only available in the BitcodeReader, for the purpose of auto-upgrades.
If you don’t control the frontend, then yes, that would have to happen in the BitcodeReader. If you’d like to make that work without a patched LLVM, then we’d probably need a hook to allow adding custom auto-upgrades with access to bitcode type ID information.
The one thing I’m worried about is the ThinLTO function instruction size import limit. In the past I’ve attempted to ignore free instructions (e.g. pointer bitcasts) when counting toward the instruction import limit and saw performance regressions, perhaps due to excessive inlining. These thresholds will probably have to be tuned. I’ll run some internal benchmarks.
I’ve been seeing some improvements and regressions with opaque pointers. Some of the (fairly large) regressions on internal benchmark, caused by a slightly simpler GEP making a loop appear cheap, causing it to be fully unrolled, causing the function to not get inlined. And in llvm-test-suite I see a regression in gesummv with -O3 due to loop unrolling not triggering (need to investigate more).
I don’t think that loop unrolling thresholds are a blocker for opaque pointers, but it’s something to note and continue to investigate.
gesummv doesn’t unroll the loop because previously we had a store of a bitcast of a gep which would bypass [1], but with opaque pointers we now see the gep directly and make the store more expensive [1].
And with the internal benchmark, previously we had
and the second gep is now considered free by [2][3], causing the full loop unrolling.
This ties into the “i8 geps everywhere” where if we want to do that, we’ll have to clean up all cost modeling of geps and find a better generic way to analyze them.
I guess adding metadata would need to be done in the (llvm 3.x) frontend which I have no control over? Or do you mean adding metadata in the BitcodeReader?