This is an archive of the discontinued LLVM Phabricator instance.

[LV][AArch64] Resolve test failure due use of unordered container
AcceptedPublic

Authored by sushgokh on Mar 6 2023, 10:30 PM.

Details

Summary

D143422 test AArch64/reg-usage.ll has an issue with the output ordering due to use of unordered container. This was discovered by
-DLLVM_REVERSE_ITERATION:BOOL=ON cmake option.

This patch tries to address it by making use of ordered container.

Diff Detail

Event Timeline

sushgokh created this revision.Mar 6 2023, 10:30 PM
sushgokh requested review of this revision.Mar 6 2023, 10:30 PM
sushgokh retitled this revision from [LV] Resolve test failure due use of unordered container to [LV][AArch64] Resolve test failure due use of unordered container.Mar 6 2023, 10:30 PM
sdesmalen accepted this revision.Mar 7 2023, 1:20 AM
This revision is now accepted and ready to land.Mar 7 2023, 1:20 AM
fhahn accepted this revision.Mar 7 2023, 4:38 AM

LGTM, thanks! The commit title could be a bit more descriptive by explicitly mentioning what the code change is, e.g. something like: [LV] Use SetVector to collect invariants in calculateRegisterUsage.

Also, this is not AArch64 specific, so it would be good to drop the [AArch64] part.

LGTM, thanks! The commit title could be a bit more descriptive by explicitly mentioning what the code change is, e.g. something like: [LV] Use SetVector to collect invariants in calculateRegisterUsage.

Also, this is not AArch64 specific, so it would be good to drop the [AArch64] part.

Thanks @fhahn for the suggestion.

Regarding inclusion of AArch64 in title, I thought this was more about AArch64 test failure. But I was incorrect. Will keep this in mind that its more about the change