This is an archive of the discontinued LLVM Phabricator instance.

[InferAddressSpaces] NFC: For noop IntToPtr/PtrToInt pair cast to operator instead of PtrToInt
ClosedPublic

Authored by rksharma on Jun 27 2021, 11:02 PM.

Details

Summary

Compiler crashes at an assertion while casting operands to PtrToIntInst at some cases when ptrtoint is present as an explicit operand to inttoptr. Explicit instruction operator as operand can not be casted to an Instruction.

This patch replaces cast from PtrToInst to Operator which are later checked for constant expressions.

Diff Detail

Event Timeline

rksharma created this revision.Jun 27 2021, 11:02 PM
rksharma requested review of this revision.Jun 27 2021, 11:02 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 27 2021, 11:02 PM
arsenm added inline comments.Jun 28 2021, 5:54 AM
llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
474

Can't you access the operator operand anyway and still handle constant expressions?

rksharma added inline comments.Jun 28 2021, 6:06 AM
llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
474

Are you saying something like,

PushPtrOperand(cast<Operator>(I2P->getOperand(0))->getOperand(0));
rksharma added inline comments.Jun 28 2021, 6:15 AM
llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
474

Yes, this will work because it does check for constant expressions inside PushPtrOperand

arsenm added inline comments.Jun 28 2021, 6:22 AM
llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
474

Yes

rksharma updated this revision to Diff 354876.Jun 28 2021, 6:29 AM
rksharma retitled this revision from [InferAddressSpaces] NFC: Check type before casting operands to PtrToIntInst to [InferAddressSpaces] NFC: For noop IntToPtr/PtrToInt pair cast to operator instead of PtrToInt .
rksharma edited the summary of this revision. (Show Details)

Updated the title, summary and the diff with casting to operator.

hliao added inline comments.Jun 28 2021, 6:43 AM
llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
474

Yes, this will work because it does check for constant expressions inside PushPtrOperand

That original cast is unnecessary and should be removed.

hliao accepted this revision.Jun 28 2021, 6:44 AM
hliao added inline comments.
llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
474

Sorry, read the wrong code. LGTM

This revision is now accepted and ready to land.Jun 28 2021, 6:44 AM
This revision was landed with ongoing or failed builds.Jun 28 2021, 6:54 AM
This revision was automatically updated to reflect the committed changes.