[ScalarEvolution] Skip values from unreachable bbs for phi ranges.
Needs ReviewPublic
Actions

Authored by fhahn on Nov 7 2022, 2:17 PM.

Details

Reviewers

nikic
efriedma
reames

Summary

Skip incoming value from unreachable blocks when computing ranges for
phis. Constructing SCEVs for values from unreachable blocks may lead to
violating assumptions based on dominance.

One example is the added LV test case. Here an assertion in GroupByComplexity
is triggered when constructing a SCEV for the incoming value from the unreachable
block because there's no dominance relation between 2 AddRecs.

It removes pr49856.ll because it is already part of
shift-recurrences.ll and needs updating.

Fixes #58811.

Diff Detail

Repository: rG LLVM Github Monorepo

Unit TestsFailed
View All

	Time	Test
	60,060 ms	x64 debian > ThreadSanitizer-x86_64.ThreadSanitizer-x86_64::restore_stack.cpp Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -fsanitize=thread -Wall -m64 -msse4.2 -gline-tables-only -I/var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/tsan/../ -std=c++11 -I/var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/tsan/../ -O1 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/tsan/restore_stack.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/tsan/X86_64Config/Output/restore_stack.cpp.tmp && not /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/tsan/X86_64Config/Output/restore_stack.cpp.tmp 2>&1 \| FileCheck /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/tsan/restore_stack.cpp

Event Timeline

fhahn created this revision.Nov 7 2022, 2:17 PM

Herald added a project: Restricted Project. · View Herald TranscriptNov 7 2022, 2:17 PM

Herald added subscribers: javed.absar, hiraditya. · View Herald Transcript

fhahn requested review of this revision.Nov 7 2022, 2:17 PM

Herald added a project: Restricted Project. · View Herald TranscriptNov 7 2022, 2:17 PM

Herald added a subscriber: pcwang-thead. · View Herald Transcript

The assertion from the bug report seems several levels removed from the top-level APIs; can we add an assertion somewhere a bit closer to the API misuse? I don't see a problem with this patch, exactly, but I'm not sure why the current code is wrong, as opposed to just being slightly suboptimal.

Harbormaster completed remote builds in B196570: Diff 473792.Nov 7 2022, 3:07 PM

Do you mind giving more details on what assumption exactly was broken?