This is an archive of the discontinued LLVM Phabricator instance.

[IR] Remove support for insertvalue constant expression
ClosedPublic

Authored by nikic on Jun 28 2022, 3:34 AM.

Details

Summary

This removes the insertvalue constant expression, as part of https://discourse.llvm.org/t/rfc-remove-most-constant-expressions/63179. This is very similar to the extractvalue removal from D125795.

ConstantExpr::getInsertValue() can be replaced with IRBuilder::CreateInsertValue() or ConstantFoldInsertValueInstruction(), depending on whether a constant result is required (with the latter being fallible).

The ConstantExpr::hasIndices() and ConstantExpr::getIndices() methods also go away here, because there are no longer any constant expressions with indices.

Diff Detail

Event Timeline

nikic created this revision.Jun 28 2022, 3:34 AM
Herald added a project: Restricted Project. · View Herald Transcript
nikic requested review of this revision.Jun 28 2022, 3:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 28 2022, 3:34 AM

Can I ask you to separate the various construction to fold changes into a separate NFC? I'd like to see that land and bake for a couple days before the removal itself goes in.

Don't we have to worry about forward compatibility with bitcode on this one? I didn't see anything for that in this change?

nikic added a comment.Jun 28 2022, 9:10 AM

Can I ask you to separate the various construction to fold changes into a separate NFC? I'd like to see that land and bake for a couple days before the removal itself goes in.

I can do that, though this wouldn't be an NFC change, as we won't create an expression for the case where the expression doesn't fold (of course, this makes little difference in practice because insertvalue pretty much always folds).

Don't we have to worry about forward compatibility with bitcode on this one? I didn't see anything for that in this change?

insertvalue is another expression not supported in bitcode. It's the last one though, other ones will need the bitcode part.

nikic planned changes to this revision.Jun 29 2022, 1:57 AM

Split off https://reviews.llvm.org/D128792 with just the changes to avoid creating expressions. Will rebase once that lands.

reames accepted this revision.Jul 1 2022, 7:39 AM

LGTM

Don't we have to worry about forward compatibility with bitcode on this one? I didn't see anything for that in this change?

insertvalue is another expression not supported in bitcode. It's the last one though, other ones will need the bitcode part.

Please update your submission comment to include this information. It's important for review - including post commit review.

This revision is now accepted and ready to land.Jul 1 2022, 7:39 AM
This revision was landed with ongoing or failed builds.Jul 4 2022, 12:27 AM
This revision was automatically updated to reflect the committed changes.
llvm/test/Assembler/insertvalue-invalid-type-1.ll