This is an archive of the discontinued LLVM Phabricator instance.

[AsmParser] Check that addrspace fits within 24 bits
ClosedPublic

Authored by luke on Dec 7 2022, 4:40 AM.

Details

Summary

Address spaces equal or larger than 1 << 24 don't fit and produce an
assertion during debug builds, or worse in release. This causes an error
to be reported during parsing instead.

Diff Detail

Event Timeline

luke created this revision.Dec 7 2022, 4:40 AM
luke requested review of this revision.Dec 7 2022, 4:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 7 2022, 4:40 AM
luke added a subscriber: asb.Dec 7 2022, 4:41 AM
luke updated this revision to Diff 480855.Dec 7 2022, 4:43 AM

Adding tests that were forgotten

Minor suggestions inline. Would you mind rebasing this on https://reviews.llvm.org/D138789 (which I will commit once approved). Should make the diff slightly smaller.

llvm/lib/AsmParser/LLParser.cpp
1824–1825

To match the logic in DataLayout.cpp. However, ideally we should have a shared constant somewhere instead of duplicating this knowledge.

luke updated this revision to Diff 480983.Dec 7 2022, 10:56 AM

Rebased upon D138789

asb added inline comments.Dec 8 2022, 4:55 AM
llvm/lib/AsmParser/LLParser.cpp
1812

Nit: I think "24-bit" would be consistent with similar strings in LLVM.

luke updated this revision to Diff 481252.Dec 8 2022, 5:13 AM
luke marked an inline comment as done.

Adjust diagnostic string

nikic added a subscriber: nikic.Dec 8 2022, 7:15 AM
nikic added inline comments.
llvm/test/Assembler/invalid-addrspace.ll
6

Does not match the updated diagnostic.

luke updated this revision to Diff 481270.Dec 8 2022, 7:17 AM

Update test with new diagnostic

luke marked an inline comment as done.Dec 8 2022, 7:17 AM
luke marked an inline comment as done.
nikic accepted this revision.Dec 8 2022, 7:18 AM

LGTM

This revision is now accepted and ready to land.Dec 8 2022, 7:18 AM
arichardson accepted this revision.Dec 8 2022, 2:48 PM
asb accepted this revision.Dec 9 2022, 1:36 AM
This revision was landed with ongoing or failed builds.Dec 9 2022, 1:54 AM
This revision was automatically updated to reflect the committed changes.