Page MenuHomePhabricator

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

Authored by sushgokh on Mon, Mar 6, 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

Unit TestsFailed

TimeTest
60,020 msx64 debian > libFuzzer.libFuzzer::minimize_crash.test
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/var/lib/buildkite-agent/builds/llvm-project/compiler-rt/lib/fuzzer -m64 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/fuzzer/NullDerefTest.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/fuzzer/X86_64DefaultLinuxConfig/Output/minimize_crash.test.tmp-NullDerefTest

Event Timeline

sushgokh created this revision.Mon, Mar 6, 10:30 PM
sushgokh requested review of this revision.Mon, Mar 6, 10:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptMon, Mar 6, 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.Mon, Mar 6, 10:30 PM
sdesmalen accepted this revision.Tue, Mar 7, 1:20 AM
This revision is now accepted and ready to land.Tue, Mar 7, 1:20 AM
fhahn accepted this revision.Tue, Mar 7, 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