This is an archive of the discontinued LLVM Phabricator instance.

[MIRVRegNamer] Avoid opcode hash collision
ClosedPublic

Authored by john.brawn on Nov 1 2022, 5:08 AM.

Details

Summary

D121929 happens to cause CodeGen/MIR/AArch64/mirnamer.mir to fail due to a hash collision caused by adding two extra opcodes. The collision is only in the top 19 bits of the hashed opcode so fix this by just using the whole hash (in fixed width hex for consistency) instead of the top 5 decimal digits.

Diff Detail

Event Timeline

john.brawn created this revision.Nov 1 2022, 5:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 1 2022, 5:08 AM
john.brawn requested review of this revision.Nov 1 2022, 5:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 1 2022, 5:08 AM
plotfi accepted this revision.Nov 1 2022, 12:10 PM
plotfi added a subscriber: aditya_nandakumar.

LGTM, Nice work John.

Would format_hex_no_prefix be replaceable with std::format once C++20 for LLVM drops?

@aditya_nandakumar Any thoughts? Are you folks still using the pass in places where the change in hashing could break things?

This revision is now accepted and ready to land.Nov 1 2022, 12:10 PM

LGTM, Nice work John.

Would format_hex_no_prefix be replaceable with std::format once C++20 for LLVM drops?

Yes, it looks like std::format with format string "{:016x}" should so the same thing as format_hex_no_prefix.

This revision was automatically updated to reflect the committed changes.