Page MenuHomePhabricator

[SDAG] Don't move DBG_VALUE instructions after insertion point during scheduling (PR53243)
ClosedPublic

Authored by nikic on Jan 17 2022, 6:53 AM.

Details

Summary

EmitSchedule() shouldn't be touching instructions after the provided insertion point. The change introduced in D83561 performs a scan to the end of the block, and thus may move unrelated instructions. In particular, this ends up moving instructions that have been produced by FastISel and will later be deleted. Moving them means that more instructions than intended are removed.

Fix this by stopping the iteration when the insertion point is reached.

Fixes https://github.com/llvm/llvm-project/issues/53243.

Diff Detail

Unit TestsFailed

TimeTest
8,140 msx64 debian > SanitizerCommon-asan-x86_64-Linux.Linux::decorate_proc_maps.cpp
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -gline-tables-only -fsanitize=address -m64 -funwind-tables -ldl -g /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/decorate_proc_maps.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Linux/Output/decorate_proc_maps.cpp.tmp
630 msx64 debian > SanitizerCommon-msan-x86_64-Linux.Linux::decorate_proc_maps.cpp
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -gline-tables-only -fsanitize=memory -m64 -funwind-tables -ldl -g /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/decorate_proc_maps.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/sanitizer_common/msan-x86_64-Linux/Linux/Output/decorate_proc_maps.cpp.tmp
890 msx64 debian > SanitizerCommon-tsan-x86_64-Linux.Linux::decorate_proc_maps.cpp
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -gline-tables-only -fsanitize=thread -m64 -funwind-tables -ldl -g /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/decorate_proc_maps.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/sanitizer_common/tsan-x86_64-Linux/Linux/Output/decorate_proc_maps.cpp.tmp

Event Timeline

nikic created this revision.Jan 17 2022, 6:53 AM
nikic requested review of this revision.Jan 17 2022, 6:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 17 2022, 6:53 AM
nikic retitled this revision from [SDAG] Don't move DBG_VALUE instructions after insertion point during scheduling to [SDAG] Don't move DBG_VALUE instructions after insertion point during scheduling (PR53243).Jan 17 2022, 6:54 AM
nikic edited the summary of this revision. (Show Details)
nikic updated this revision to Diff 400546.Jan 17 2022, 7:23 AM

Swap order of checks. We should stop at the insertion point even if it doesn't happen to be DBG_VALUE.

ormris removed a subscriber: ormris.Jan 18 2022, 9:39 AM
fhahn accepted this revision.Jan 22 2022, 2:40 PM

LGTM, thanks

llvm/test/CodeGen/X86/pr53243-tail-call-fastisel.ll
29

nit: most of those namespaces/enumerates should not be needed I think.

43

nit: most of the types shouldn't be needed.

This revision is now accepted and ready to land.Jan 22 2022, 2:40 PM
This revision was landed with ongoing or failed builds.Jan 24 2022, 1:50 AM
This revision was automatically updated to reflect the committed changes.
nikic marked an inline comment as done.