[X86][CostModel] Strip bitcasts when looking for store GEP operand
AbandonedPublic
Actions

Authored by aeubanks on Apr 1 2022, 11:21 AM.

Details

Reviewers

RKSimon
ABataev

Summary

This heuristic was introduced in D35888. Assuming it is correct, this
patch should make cost modeling a bit more robust.

This came about when looking at opaque pointers where a bitcast that previously appeared hid this heuristic. This actually regresses the benchmark I was looking at due to less loop unrolling (gesummv in llvm-test-suite), but I'm not sure how else to solve that besides removing this heuristic entirely.

Diff Detail

Repository: rG LLVM Github Monorepo

Unit TestsFailed
View All

	Time	Test
	60,040 ms	x64 debian > MLIR.Examples/standalone::test.toy Script: -- : 'RUN: at line 1'; /usr/bin/cmake /var/lib/buildkite-agent/builds/llvm-project/mlir/examples/standalone -G "Ninja" -DCMAKE_CXX_COMPILER=/usr/bin/clang++ -DCMAKE_C_COMPILER=/usr/bin/clang -DLLVM_ENABLE_LIBCXX=OFF -DMLIR_DIR=/var/lib/buildkite-agent/builds/llvm-project/build/lib/cmake/mlir ; /usr/bin/cmake --build . --target check-standalone \| tee /var/lib/buildkite-agent/builds/llvm-project/build/tools/mlir/test/Examples/standalone/Output/test.toy.tmp \| /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/mlir/test/Examples/standalone/test.toy
	60,040 ms	x64 debian > libFuzzer.libFuzzer::large.test Script: -- : 'RUN: at line 3'; /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/LargeTest.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/fuzzer/X86_64DefaultLinuxConfig/Output/large.test.tmp-LargeTest

Event Timeline

aeubanks created this revision.Apr 1 2022, 11:21 AM

Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2022, 11:21 AM

Herald added subscribers: pengfei, zzheng, hiraditya. · View Herald Transcript

aeubanks requested review of this revision.Apr 1 2022, 11:21 AM

Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2022, 11:21 AM

Herald added a subscriber: llvm-commits. · View Herald Transcript

aeubanks edited the summary of this revision. (Show Details)Apr 1 2022, 11:24 AM

aeubanks added a reviewer: RKSimon.

Harbormaster completed remote builds in B157461: Diff 419815.Apr 1 2022, 11:56 AM

RKSimon added inline comments.Apr 7 2022, 3:54 AM

llvm/lib/Target/X86/X86TargetTransformInfo.cpp
3910	I wonder whether this needs inverting - and we default stores to a cost of TTI::TCC_Basic * 2 UNLESS after stripping we find all indices are constant?

nikic added a subscriber: nikic.Apr 7 2022, 5:47 AM

nikic added inline comments.

llvm/lib/Target/X86/X86TargetTransformInfo.cpp
3910	I believe this code is intended to catch the case where we have a GEP that was modeled as free due to addressing mode support, but has non-constant indices and that addressing mode is non-free in store instructions. So I think the current logic is right, though could be more precise if we checked that the GEP cost is actually TCC_Free, otherwise we might double-cost the GEP.