This is an archive of the discontinued LLVM Phabricator instance.

TableGen: sort SearchableTable emission order by fields, not top-level name
ClosedPublic

Authored by t.p.northover on Nov 2 2021, 4:22 AM.

Details

Summary

This is often used for anonymous definitions, so we were sorting alphabetically by anonymous_1234 record names, which while less bad than pointers can be easily perturbed by adding code even in completely unrelated systems.

That causes test failures on AArch64 when sysregs with multiple valid names suddenly start printing a different one for no obvious reason.

At Apple we've had aliased sysregs for a while, and this semi-deterministic printing was a scourge ; and judging by a few of the check lines being replaced it was hit even during the development of D110065.

So this patch instead sorts by the contents of the record.

Diff Detail

Event Timeline

t.p.northover created this revision.Nov 2 2021, 4:22 AM
t.p.northover requested review of this revision.Nov 2 2021, 4:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 2 2021, 4:22 AM

Looks good. Thanks for fixing this inconsistency. Can you remove the obsolete CHECK lines and replace two more occurences of {{PRLAR[0]?_EL2}} in the tests?

llvm/test/MC/AArch64/armv8r-sysreg.s
313–316

obsolete

365

PRLAR0_EL2 ?

388–391

obsolete

440

PRLAR0_EL2 ?

463–466

obsolete

538–541

obsolete

chill added a subscriber: chill.Nov 12 2021, 3:15 AM

May be this https://reviews.llvm.org/D74074 is not needed anymore, as long as reverting the change to
AArch64InstPrinter.cpp alone does not cause failure in tests?

t.p.northover accepted this revision.Jan 18 2022, 5:00 AM
t.p.northover marked 6 inline comments as done.

Thanks, I've committed it with the updated tests you suggested as 51f743db. Well, I think it's probably still worth checking we can assemble PRBAR_EL1 etc, even if they're not printed out again the same so I left those in for now.

This revision is now accepted and ready to land.Jan 18 2022, 5:00 AM
t.p.northover closed this revision.Jan 18 2022, 5:00 AM