Skip to content

Conversation

@trbabb
Copy link
Contributor

@trbabb trbabb commented Mar 21, 2021

New feature: gep instructions can accept Python integers as indices, instead of requiring the client code to tediously construct ir.Constants around each index.

For example:

int32 = ir.IntType(32)
builder.gep(my_object, [int32(5), int32(0), int32(3)])

becomes

builder.gep(my_object, [5, 0, 3])

The width of each converted integer will be chosen to be the smallest number of 8-bit bytes that is a power of two and can represent the value.

It is permissible to intermix Python integers with IR instructions in the index list.

trbabb added 2 commits March 21, 2021 14:32
ir.Constants will be automatically constructed around Python integers if they are provided.
The width of each converted integer will be chosen to be the smallest number of 8-bit bytes that can represent the value.
This changes the scheme to use i32s for all GEP indicies converted from Python ints, unless the index is too large to represent with an i32, in which case a wider integer is used.
@esc esc added this to the Version 0.37.0 RC milestone Mar 22, 2021
@esc
Copy link
Member

esc commented Mar 22, 2021

@trbabb thank you for submitting this to the Numba issue tracker. I have added this to the 0.37.0 milestone. Also, it seems like there are some remaining flake8 issues to be resolved.

@trbabb
Copy link
Contributor Author

trbabb commented Mar 22, 2021

Fixed the stray whitespace, but the overlong line is inlined SSA, which that file is already full of, and which I can't really truncate. What do I need to do to prevent that from failing the test?

@esc
Copy link
Member

esc commented Mar 22, 2021

Fixed the stray whitespace, but the overlong line is inlined SSA, which that file is already full of, and which I can't really truncate. What do I need to do to prevent that from failing the test?

This should help: https://flake8.pycqa.org/en/3.1.1/user/ignoring-errors.html#in-line-ignoring-errors

Copy link
Member

@gmarkall gmarkall left a comment

Choose a reason for hiding this comment

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

Many thanks for the PR!

I like the idea of this change, but I think the changes to the tests need modifying. Instead of modifying an existing test case, could you add a new test case that tests accepting integers for the gep instruction (and a mix of integers and ir.Constant values) please?

Comment on lines +501 to +502
i_type = types.IntType(bits)
i = i_type(i)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
i_type = types.IntType(bits)
i = i_type(i)
i = types.IntType(bits)(i)

@gmarkall
Copy link
Member

@trbabb Many thanks for the PR and your efforts so far!

I'm spending some time going through the llvmlite PR backlog, and I have a couple of questions on this PR:

  • Do you plan to continue with this PR? If it's difficult for you to work on this PR for any reason, please let me know and we can look into whether / how to proceed further.
  • If you're keen to continue - do you need any further advice on how to proceed?

@sklam sklam modified the milestones: 0.39.0 RC, PR Backlog Jun 1, 2022
@esc esc removed this from the PR Backlog milestone Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants