Phi instructions should invalidate their string cache when new incomings are added

[trbabb]

Stringifying a phi (including for debugging purposes) can cause them to freeze themselves in a stale state, and subsequently added incoming instructions will be incorrectly omitted from the instruction.

Any time a phi is mutated by adding an incoming expression, its string cache should be immediately invalidated.

[jceipek]

Here's a concrete example of where this could cause a problem:

print("before:", entry_phi) # before: %".7" = phi i64 [5, %"entry"]
entry_phi.add_incoming(v, body_blk) # This isn't represented in the final output because the line above populates the cache
print("after:", entry_phi) # after: %".7" = phi i64 [5, %"entry"]

Leads to this compile error:

compile error
Traceback (most recent call last):
  File "./codexec", line 169, in handle_object_code
    self.module.verify()
  File "/opt/venv/lib/python3.8/site-packages/llvmlite/binding/module.py", line 115, in verify
    raise RuntimeError(str(outmsg))
RuntimeError: PHINode should have one entry for each predecessor of its parent basic block!
  %.7 = phi i64 [ 5, %entry ]

Workaround:

print("before:", entry_phi) # before: %".7" = phi i64 [5, %"entry"]
entry_phi.add_incoming(v, body_blk)
entry_phi._clear_string_cache() # Clearing the cache manually ensures the above line is represented in the output
print("after:", entry_phi) # after: %".7" = phi i64 [5, %"entry"], [%".11", %"loop_body_b85hSr3tU4j6E/Vr"]

[esc]

@trbabb thank you for submitting this. I have added it to the queue for review.

[trbabb]

Any chance this and #710 could make it into the next release? It'd really help clean up some workarounds! Thanks!

[gmarkall]

@trbabb Many thanks for the PR, and apologies for the delay in its review. I'm spending some time going through the llvmlite PR backlog so I'm hoping to help move this forward now.

The changes seem like a good idea in general - as a general question (not necessarily needing to be addressed in this PR) are there other instructions that should invalidate their string cache when modified?

Could you please add a test of the modified functionality? (Perhaps based on @jceipek's example above).