Add IR layer typed pointers mode #1159

rj-jesus · 2025-02-19T11:04:04Z

This patch adds preliminary support for a "lazy opaque pointers" option whereby we emit IR with typed pointers if pointee types are known (even if opaque pointers are enabled). This makes numba-cuda work with opaque pointers enabled and should unblock #1082.

I've enabled opaque pointers unconditionally and made lazy opaque pointers on by default to facilitate testing. These changes could (should?) be moved to a separate PR if required.

gmarkall · 2025-02-19T12:50:28Z

llvmlite/ir/types.py

@@ -190,17 +186,6 @@ def gep(self, i):
    def intrinsic_name(self):
        return 'p%d%s' % (self.addrspace, self.pointee.intrinsic_name)

-    @classmethod


This was added by @sklam in #1051. I think there's no way this can survive the transition to opaque pointers anyway (and the fact it doesn't seem to have an effect on tests is also somewhat reassuring), but I want to check if @sklam has any thoughts here.

The opaque pointer version of this shouldn't be affected by this patch (which is why the tests aren't affected either).
I think the typed version can't survive, but my understanding from #1064 (comment) was that this was okay.

Ok to remove.

llvmlite/__init__.py

gmarkall

Thanks for this @rj-jesus !

I think the general idea is good and I'd be keen to press on with it. I'd hope our strategy would be something like:

Merge this PR
Get comfortable that nothing Numba-related is broken by it
Bump the minimum LLVM version to 16 (because it already builds with it anyway, this should be a simple bump)
Remove the references to typed pointers in the binding layer to clean things up a bit
Move to later LLVM versions at our convenience

The only idea I'd like to float is WRT naming - we have "opaque pointers" and "lazy opaque pointers" which on the face of it don't have much specific meaning. I'd like to propose that:

What we're adding here is called "IR layer typed pointers" (or "IR layer opaque pointers", with the required inversion of boolean conditions)
What is enabled by the _opaque_pointers_enabled() function is called "Binding layer opaque pointers"

I think this naming makes it a bit clearer where the effects and differences lie in terms of the abstractions presented by llvmlite. I would be keen to hear the perspective of @sklam and @stuartarchibald here too though.

rj-jesus · 2025-02-19T14:43:24Z

Hi @gmarkall, thanks very much for the feedback!

Your strategy sounds very good to me.

On the naming issue, I'm more than happy to rename "lazy opaque pointers" to "IR layer opaque pointers" or something else entirely, "lazy opaque pointers" was just a work-in-progress name in any case! As far as the normal "opaque pointer mode" goes, while I'm also very happy to rename it, I think maybe we should just remove it entirely. It won't be an option per se going forward as there will be no alternative, and it was only introduced in the first place as a stop-gap solution to enable testing and porting. We've had it for a release cycle, which I believe was the initial plan. What do you think?

Co-authored-by: Graham Markall <[email protected]>

gmarkall · 2025-02-19T15:43:23Z

As far as the normal "opaque pointer mode" goes, while I'm also very happy to rename it, I think maybe we should just remove it entirely. It won't be an option per se going forward as there will be no alternative, and it was only introduced in the first place as a stop-gap solution to enable testing and porting. We've had it for a release cycle, which I believe was the initial plan. What do you think?

I think this is a good idea, and it would save an iteration compared to my original plan - I prefer what you suggest instead.

gmarkall · 2025-02-19T15:43:57Z

On the naming issue, I'm more than happy to rename "lazy opaque pointers" to "IR layer opaque pointers" or something else entirely, "lazy opaque pointers" was just a work-in-progress name in any case!

Thanks... Will wait and see what others think of the naming.

At the LLVM level, typed pointers are no longer supported going forward. This ticket removes typed pointer support at the LLVM binding layer and bumps the LLVM release used to LLVM 16. This builds on top of numba#1159.

rj-jesus · 2025-02-20T12:21:27Z

Hi @gmarkall, as discussed I've raised #1160 to drop the old "binding layer" opaque pointers mode (and bump the LLVM release supported to 16), and I've also just committed the rename "lazy opaque pointers" to "IR layer typed pointers" you had suggested. Can you think of anything else we should be doing?

gmarkall · 2025-02-20T12:36:59Z

Can you think of anything else we should be doing?

Not at this point - thanks for the updates!

gmarkall · 2025-02-25T15:41:19Z

From triage meeting - I'll continue to review this until it's RTM.

gmarkall

The code changes look good to me, so I'm approving this.

However, there are updates needed for the docs, examples, and test scripts, which I'll push after approving for @rj-jesus to review (and CI to test 🙂).

rj-jesus

Thank you very much for these changes @gmarkall! Everything looks good, I just left a few comments about some of the phrasing, but nothing special.

I can't approve this explicitly, but this generally looks good to me.

rj-jesus · 2025-02-28T15:39:06Z

docs/source/user-guide/deprecation.rst

+Typed Pointers by default with LLVM 15 in llvmlite 0.44.
+
+The binding layer will move to using Opaque Pointers. The IR layer will still
+support both Typed and Opaque Pointers, defaulting to Typed Pointers. This


Maybe "defaulting to Typed Pointers when pointee types are provided"?

Typed pointers are effectively "best-effort", even with IR layer typed pointers enabled.
Currently, nothing prevents the user from mixing opaque and typed pointers.

rj-jesus · 2025-02-28T15:41:55Z

docs/source/user-guide/deprecation.rst

-pointers will break if not modified to use opaque pointers.
+In a future release of llvmlite, code that uses llvmlite to work with pointers
+or to parse assembly that uses pointers will break if not modified to use
+Opaque Pointers.


I think this depends on what the user does with the pointers. llvmlite should happily parse IR with pointers (typed or not); it's just that, going forward, it will always represent these pointers as opaque pointers internally.

What I was aiming to imply here was that we might remove typed pointer support from the IR layer, but without being specific about exactly when. However, I think the "parse assembly that uses pointers" part of the sentence is making this overall statement wrong / misleading... Will see what I can do to improve it.

Maybe we can be explicit, for example with something along the lines of:

In a future release of llvmlite, code that uses llvmlite to work with typed pointers or to generate IR with typed pointers will no longer be supported.

Do you think something along those lines would help make the intent clearer?

rj-jesus · 2025-02-28T15:42:42Z

docs/source/user-guide/deprecation.rst

- In llvmlite 0.44, Opaque Pointers will be the default pointer kind.
- In llvmlite 0.45, support for Typed Pointers will be removed.
+- In llvmlite 0.45, support for Opaque Pointers in the binding layer will be
+  removed. The IR layer will still use Typed Pointers by default.


I think you meant "support for Typed Pointers in the binding layer will be removed"?

rj-jesus · 2025-02-28T16:00:16Z

docs/source/user-guide/ir/types.rst


   # ... continue using llvmlite ...

-When Opaque Pointers are enabled, the pointer type is represented using:
+When Opaque Pointers are enabled (by disabling Typed Pointers), the pointer
+type is represented using:


Whilst this is technically true, typed pointers won't be represented as "true" opaque pointers in the sense that they'll still be represented as _TypedPointerType regardless of ir_layer_typed_pointers_enabled.

This shouldn't matter for users of PointerType as everything should still work transparently (that's the whole point of having the shim). For all intended purposes, all pointer types are always PointerType.

rj-jesus · 2025-02-28T16:12:43Z

docs/source/user-guide/ir/types.rst

-Pointers are currently the default; Opaque Pointers will become the default in
-future, and support for Typed Pointers will be eventually be removed.
+The IR layer presently supports both *Typed Pointers* and *Opaque Pointers*.
+Typed Pointers are currently the default; Opaque Pointers will become the


Here, "the default" effectively depends on whether the pointee type is known. It's not so much of an option; it depends on how the user creates the pointer types.

rj-jesus added 2 commits February 19, 2025 10:50

Add lazy opaque pointers mode

b261919

Fix test_ir tests

ff60f61

gmarkall added the 3 - Ready for Review label Feb 19, 2025

gmarkall reviewed Feb 19, 2025

View reviewed changes

llvmlite/__init__.py Outdated Show resolved Hide resolved

gmarkall reviewed Feb 19, 2025

View reviewed changes

llvmlite/__init__.py Outdated Show resolved Hide resolved

gmarkall requested changes Feb 19, 2025

View reviewed changes

gmarkall added 4 - Waiting on author 4 - Waiting on second reviewer and removed 3 - Ready for Review labels Feb 19, 2025

rj-jesus and others added 2 commits February 19, 2025 14:48

Update llvmlite/__init__.py

dba9682

Co-authored-by: Graham Markall <[email protected]>

Update llvmlite/__init__.py

d8d6fe6

Co-authored-by: Graham Markall <[email protected]>

rj-jesus mentioned this pull request Feb 20, 2025

Drop binding layer typed pointer support and move to LLVM 16. #1160

Open

Rename lazy opaque pointers to IR layer typed pointers.

16c1b48

rj-jesus changed the title ~~Add lazy opaque pointers mode~~ Add IR layer typed pointers mode Feb 20, 2025

gmarkall added 4 - Waiting on reviewer and removed 4 - Waiting on author labels Feb 20, 2025

gmarkall removed the 4 - Waiting on second reviewer label Feb 25, 2025

gmarkall added 4 commits February 28, 2025 12:21

Update deprecation notice for IR layer typed pointers

be48e7b

Update types documentation for IR layer typed pointers

eca2f5e

Update examples for IR layer typed pointers

fac2b28

Update test scripts for IR layer opaque pointers

01e1669

gmarkall previously approved these changes Feb 28, 2025

View reviewed changes

gmarkall dismissed their stale review via 01e1669 February 28, 2025 12:46

gmarkall added 4 - Waiting on second reviewer 4 - Waiting on CI and removed 4 - Waiting on reviewer 4 - Waiting on CI labels Feb 28, 2025

rj-jesus commented Feb 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add IR layer typed pointers mode #1159

Add IR layer typed pointers mode #1159

rj-jesus commented Feb 19, 2025

gmarkall Feb 19, 2025

rj-jesus Feb 19, 2025

sklam Feb 25, 2025

gmarkall left a comment

rj-jesus commented Feb 19, 2025

gmarkall commented Feb 19, 2025

gmarkall commented Feb 19, 2025

rj-jesus commented Feb 20, 2025

gmarkall commented Feb 20, 2025

gmarkall commented Feb 25, 2025

gmarkall left a comment

rj-jesus left a comment

rj-jesus Feb 28, 2025

rj-jesus Feb 28, 2025

gmarkall Feb 28, 2025

rj-jesus Feb 28, 2025

rj-jesus Feb 28, 2025

rj-jesus Feb 28, 2025

rj-jesus Feb 28, 2025

Add IR layer typed pointers mode #1159

Are you sure you want to change the base?

Add IR layer typed pointers mode #1159

Conversation

rj-jesus commented Feb 19, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmarkall left a comment

Choose a reason for hiding this comment

rj-jesus commented Feb 19, 2025

gmarkall commented Feb 19, 2025

gmarkall commented Feb 19, 2025

rj-jesus commented Feb 20, 2025

gmarkall commented Feb 20, 2025

gmarkall commented Feb 25, 2025

gmarkall left a comment

Choose a reason for hiding this comment

rj-jesus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment