Drop the JointMatrixINTEL struct-renaming pass when opaque pointers are enabled. #1570
Conversation
…re enabled. The frontend is being changed to lower the struct name to the correct LLVM name directly, obviating the need for this check. See intel/llvm#6535 for this change. This marks the removal of the final call to the deprecated method Type::getPointerElementType, although there remains some code that is not fully working with opaque pointers enabled.
Co-authored-by: Dmitry Sidorov <[email protected]>
Co-authored-by: Dmitry Sidorov <[email protected]>
| // "%spirv.JointMatrixINTEL._{parameters}" directly, obviating the need for | ||
| // this check. |
There was a problem hiding this comment.
I suppose we should add a test with new LLVM IR representation?
BTW, do we need to remove the old one or we still consider that we should be/are able to accept the old IR as well?
There was a problem hiding this comment.
My plan is to maintain compatibility with the old IR for a short while (perhaps until LLVM rips out support for PointerType::getPointerElementType entirely).
As for tests, I'm somewhat holding off on that until I (or somebody else) starts going through and moving everything to use opaque pointers in lieu of typed pointers.
There was a problem hiding this comment.
Considering that we don't test non-opaque pointers path at all (if I understand correctly), I guess it is ok to merge it as-is for now and push some fixes if they would be necessary later.
I will approve this patch, but it would be good if someone else confirmed that it is ok for such patches to come without tests for now. (Pardon my ignorance, I haven't been following the development in the translator for the past few months)
There was a problem hiding this comment.
it would be good if someone else confirmed that it is ok for such patches to come without tests for now
I think that's the most practical thing we can do while we are in this transitional state, and it is pretty much how we've handled the transition so far. The expectation is indeed that further fixes will be needed when we start converting the test suite to opaque pointers.
The frontend is being changed to lower the struct name to the correct LLVM name
directly, obviating the need for this check. See
intel/llvm#6535 for this change.
This marks the removal of the final call to the deprecated method
Type::getPointerElementType, although there remains some code that is not fully
working with opaque pointers enabled.