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

Add IR layer typed pointers mode #1159

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions buildscripts/incremental/test.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
call activate %CONDA_ENV%

if "%OPAQUE_POINTERS%"=="yes" (
set LLVMLITE_ENABLE_OPAQUE_POINTERS=1
echo "Testing with opaque pointers enabled"
set LLVMLITE_ENABLE_IR_LAYER_TYPED_POINTERS=0
echo "Testing with IR layer opaque pointers enabled"
) else (
echo "Testing with opaque pointers disabled"
echo "Testing with IR layer opaque pointers disabled"
)

python runtests.py -v
6 changes: 3 additions & 3 deletions buildscripts/incremental/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ set -v -e
python --version

if [ "$OPAQUE_POINTERS" == "yes" ]; then
export LLVMLITE_ENABLE_OPAQUE_POINTERS=1
echo "Testing with opaque pointers enabled"
export LLVMLITE_ENABLE_IR_LAYER_TYPED_POINTERS=0
echo "Testing with IR layer opaque pointers enabled"
else
echo "Testing with opaque pointers disabled"
echo "Testing with IR layer opaque pointers disabled"
fi

if [ "$WHEEL" == "yes" ]; then
Expand Down
33 changes: 26 additions & 7 deletions docs/source/user-guide/deprecation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,38 @@ switching to `Opaque Pointers <https://llvm.org/docs/OpaquePointers.html>`_:
therefore may have bugs that go unfixed).
- In LLVM 17, support for Typed Pointers is removed.

Although Opaque Pointers are already the default in LLVM 15, llvmlite still uses
Typed Pointers by default with LLVM 15.
Although Opaque Pointers are already the default in LLVM 15, llvmlite still used
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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

allows llvmlite to continue being used with LLVM-based projects that use an
older LLVM version, such as `NVVM
<https://docs.nvidia.com/cuda/nvvm-ir-spec/>`_.

Examples(s) of the impact
-------------------------

Code that uses llvmlite to work with pointers or to parse assembly that uses
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.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?


In the meantime, IR generated by the IR layer and parsed by the binding layer
will be auto-upgraded to use Opaque Pointers transparently; no action is
required by the user.

Schedule
--------

- 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.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

- In a future version of llvmlite (>= 0.46), the IR layer will use Opaque
Pointers by default.
- In a subsequent version of llvmlite (>= 0.47), support for Typed Pointers at
the IR layer will be removed.

This schedule may be updated to push out the deprecation / removal of Typed
Pointer support to still later versions of llvmlite.

Recommendations
---------------
Expand Down Expand Up @@ -127,7 +145,8 @@ When working with global variables and functions (which will be :class:`ValueRef

When passing assembly to :func:`llvmlite.binding.parse_assembly`:

- Ensure that any IR passed to ``parse_assembly()`` uses Opaque Pointers.
- IR passed to ``parse_assembly()`` is free to use either Typed or Opaque
Pointers.


Deprecation of `llvmlite.llvmpy` module
Expand Down
19 changes: 11 additions & 8 deletions docs/source/user-guide/ir/types.rst
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,10 @@ Atomic types
Pointer Types
=============

llvmlite presently supports both *Typed Pointers* and *Opaque Pointers*. Typed
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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

default in future, and support for Typed Pointers will be eventually be
removed.

.. note::
Further details of the migration to Opaque Pointers are outlined in the
Expand All @@ -126,23 +127,25 @@ When Typed Pointers are enabled, the pointer type is represented using:

The type pointed to.

Opaque pointers can be enabled by setting the environment variable:
Opaque Pointers can be enabled in the IR layer by setting the environment
variable:

.. code:: bash

LLVMLITE_ENABLE_OPAQUE_POINTERS=1
LLVMLITE_ENABLE_IR_LAYER_TYPED_POINTERS=0

or by setting the ``opaque_pointers_enabled`` attribute after importing
or by setting the ``ir_layer_typed_pointers_enabled`` attribute after importing
llvmlite, but prior to using any of its functionality. For example:

.. code:: python

import llvmlite
llvmlite.opaque_pointers_enabled = True
llvmlite.ir_layer_typed_pointers_enabled = False

# ... 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:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.


.. class:: PointerType(addrspace=0)
:no-index:
Expand Down
2 changes: 1 addition & 1 deletion examples/opaque_pointers/llvmir.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import llvmlite
llvmlite.opaque_pointers_enabled = True
llvmlite.ir_layer_typed_pointers_enabled = False

import llvmlite.ir as ll

Expand Down
2 changes: 1 addition & 1 deletion examples/opaque_pointers/sum.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
pass

import llvmlite
llvmlite.opaque_pointers_enabled = True
llvmlite.ir_layer_typed_pointers_enabled = False

import llvmlite.ir as ll
import llvmlite.binding as llvm
Expand Down
12 changes: 8 additions & 4 deletions llvmlite/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,12 @@
del get_versions

# FIXME: Remove me once typed pointers are no longer supported.
def _opaque_pointers_enabled():
# Opaque pointers unconditionally required for later LLVM versions
opaque_pointers_enabled = True
# We default to IR layer typed pointers being enabled, since they're needed in
# the most common usage scenarios with later LLVMs.
def _ir_layer_typed_pointers_enabled():
import os
return os.environ.get('LLVMLITE_ENABLE_OPAQUE_POINTERS', '0') == '1'
opaque_pointers_enabled = _opaque_pointers_enabled()
del _opaque_pointers_enabled
return os.environ.get('LLVMLITE_ENABLE_IR_LAYER_TYPED_POINTERS', '1') == '1'
ir_layer_typed_pointers_enabled = _ir_layer_typed_pointers_enabled()
del _ir_layer_typed_pointers_enabled
19 changes: 2 additions & 17 deletions llvmlite/ir/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@

import struct

from llvmlite import ir_layer_typed_pointers_enabled
from llvmlite.ir._utils import _StrCaching

# FIXME: Remove me once typed pointers are no longer supported.
from llvmlite import opaque_pointers_enabled


def _wrapname(x):
return '"{0}"'.format(x.replace('\\', '\\5c').replace('"', '\\22'))
Expand Down Expand Up @@ -145,8 +143,6 @@ def from_llvm(cls, typeref, ir_ctx):
"""
Create from a llvmlite.binding.TypeRef
"""
if not opaque_pointers_enabled:
return _TypedPointerType.from_llvm(typeref, ir_ctx)
return cls()


Expand All @@ -163,7 +159,7 @@ def __init__(self, pointee, addrspace=0):
self.is_opaque = False

def _to_string(self):
if not opaque_pointers_enabled:
if ir_layer_typed_pointers_enabled:
return "{0}*".format(self.pointee) if self.addrspace == 0 else \
"{0} addrspace({1})*".format(self.pointee, self.addrspace)
return super(_TypedPointerType, self)._to_string()
Expand All @@ -190,17 +186,6 @@ def gep(self, i):
def intrinsic_name(self):
return 'p%d%s' % (self.addrspace, self.pointee.intrinsic_name)

@classmethod
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

Ok to remove.

def from_llvm(cls, typeref, ir_ctx):
"""
Create from a llvmlite.binding.TypeRef
"""
assert not opaque_pointers_enabled
# opaque pointer will change this
[pointee] = typeref.elements
# addrspace is not handled
return cls(pointee.as_ir(ir_ctx=ir_ctx))


class VoidType(Type):
"""
Expand Down
Loading
Loading