added ability to iterate over basic block's predecessors#776
added ability to iterate over basic block's predecessors#776carpall wants to merge 3 commits intonumba:mainfrom
Conversation
|
There's a problem with tests I think, the change I made has no side effects, it can't make crash tests |
gmarkall
left a comment
There was a problem hiding this comment.
Many thanks for the PR!
I'm finding the API confusing - wouldn't it be more straightforward if blocks had a method to add predecssors, rather than the IRBuilder having a set_pred method that sets the predecessor of the passed block as the current block? (Does the C API do things this way?)
Could you also include tests and documentation for the additional feature please?
|
@carpall thank you for submitting this! It seems like |
I second that, thank you @gmarkall -- we will need some tests. I do like the feature though. |
how can I add documentation for this little feature? I didn't find something about contributions on the readme |
https://llvmlite.readthedocs.io/en/latest/contributing.html <-- maybe a bit out of date, unfortunately. |
Now I have no time to edit the documentation, about the test I think it was so small as change to be tested entry:
br label %bb0
bb0:
br i1 %0, label %bb1, label %bb2 ; bb1.predecessors: [bb0], bb2.predecessors: [bb0], bb0.predecessors: [entry] |
|
Many thanks for the update!
No problem - when you do have time, you can update this PR and ping either myself or @esc for us to review the new changes please?
In general all changes need to be tested - this helps in multiple ways:
If you'd like to proceed with the PR (adding tests and documentation), could you please also respond to the question about the API design from above? This was:
Many thanks for your efforts - do let us know if you need any further info / clarification. |
Like in the C api, basic blocks have a list of predecessors.
Predecessors are all the basic blocks which refer to a specific basic block.
For example: