Page MenuHomePhabricator

[TTI] NFC: Change getNumberOfParts to return InstructionCost.
Needs ReviewPublic

Authored by dfukalov on May 31 2021, 6:39 AM.



This patch migrates the TTI cost interfaces to return an InstructionCost.

See this patch for the introduction of the type:
See this thread for context:

Diff Detail

Unit TestsFailed

9,810 msx64 debian > libarcher.races::lock-unrelated.c
Script: -- : 'RUN: at line 13'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -fopenmp -pthread -fno-experimental-isel -g -O1 -fsanitize=thread -I /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests -I /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/runtime/src -L /var/lib/buildkite-agent/builds/llvm-project/build/lib -Wl,-rpath,/var/lib/buildkite-agent/builds/llvm-project/build/lib /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests/races/lock-unrelated.c -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/lock-unrelated.c.tmp -latomic && env TSAN_OPTIONS='ignore_noninstrumented_modules=0:ignore_noninstrumented_modules=1' /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests/deflake.bash /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/lock-unrelated.c.tmp 2>&1 | tee /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/lock-unrelated.c.tmp.log | /var/lib/buildkite-agent/builds/llvm-project/build/./bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests/races/lock-unrelated.c

Event Timeline

dfukalov created this revision.May 31 2021, 6:39 AM
dfukalov requested review of this revision.May 31 2021, 6:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 31 2021, 6:39 AM

This change doesn't make much sense to me, getNumberOfParts returns how many pieces to split up the provided type. If the answer is unknown the value 0 is used for that, so there shouldn't be a reason why this would ever need to return Invalid.
Additionally, all current uses of this function are in the SLPVectorizer and the SystemZ target where vector types are fixed, so it should always be possible to split a type in a known number of parts.

Yes, using InstructionCost as number of parts may be confusing, but

  1. The function is also used in line 7245 of LoopVectorize.cpp so "Transforms/LoopVectorize/AArch64/scalable-vf-hint.ll" test starts to assert in @test_no_sve just after getTypeLegalizationCost() returns invalid cost.
  2. The same logic was used for getRegUsageForType() in D102541 for number of registers so I decided to repeat it here.

It seemed to me that zero result of getRegUsageForType() may be correct for some target, but I'm not sure we need it.

Possible it should be better to convert 'invalid' case to 'zero' result here.
If so, I guess should we change getRegUsageForType() to the same logic and return there unsigned too (zero for invalid cost)?