Page MenuHomePhabricator

[BasicTTI] Set scalarization cost of getCommonMaskedMemoryOpCost to Invalid.
Needs ReviewPublic

Authored by Jim on Feb 11 2022, 1:18 AM.

Details

Reviewers
sdesmalen
Summary

This fixes crashing on auto *VT = cast<FixedVectorType>(DataTy); when
the type is a scalable vector.

Fix https://github.com/llvm/llvm-project/issues/53599.

Diff Detail

Unit TestsFailed

TimeTest
60,100 msx64 debian > Clang.CodeGen/RISCV/rvv-intrinsics::vloxseg.c
Script: -- : 'RUN: at line 3'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/15.0.0/include -nostdsysteminc -triple riscv64 -target-feature +f -target-feature +d -target-feature +zfh -target-feature +v -disable-O0-optnone -emit-llvm /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/RISCV/rvv-intrinsics/vloxseg.c -o - | /var/lib/buildkite-agent/builds/llvm-project/build/bin/opt -S -mem2reg | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck --check-prefix=CHECK-RV64 /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/RISCV/rvv-intrinsics/vloxseg.c
60,060 msx64 debian > Clang.CodeGen/RISCV/rvv-intrinsics::vlseg.c
Script: -- : 'RUN: at line 3'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/15.0.0/include -nostdsysteminc -triple riscv32 -target-feature +f -target-feature +d -target-feature +v -target-feature +zfh -disable-O0-optnone -fallow-half-arguments-and-returns -emit-llvm /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/RISCV/rvv-intrinsics/vlseg.c -o - | /var/lib/buildkite-agent/builds/llvm-project/build/bin/opt -S -mem2reg | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck --check-prefix=CHECK-RV32 /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/RISCV/rvv-intrinsics/vlseg.c
60,060 msx64 debian > Clang.CodeGen/RISCV/rvv-intrinsics::vlsegff.c
Script: -- : 'RUN: at line 3'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/15.0.0/include -nostdsysteminc -triple riscv32 -target-feature +f -target-feature +d -target-feature +v -target-feature +zfh -disable-O0-optnone -fallow-half-arguments-and-returns -emit-llvm /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/RISCV/rvv-intrinsics/vlsegff.c -o - | /var/lib/buildkite-agent/builds/llvm-project/build/bin/opt -S -mem2reg | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck --check-prefix=CHECK-RV32 /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/RISCV/rvv-intrinsics/vlsegff.c
60,090 msx64 debian > Clang.CodeGen/RISCV/rvv-intrinsics::vluxseg.c
Script: -- : 'RUN: at line 3'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/15.0.0/include -nostdsysteminc -triple riscv64 -target-feature +f -target-feature +d -target-feature +zfh -target-feature +v -disable-O0-optnone -emit-llvm /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/RISCV/rvv-intrinsics/vluxseg.c -o - | /var/lib/buildkite-agent/builds/llvm-project/build/bin/opt -S -mem2reg | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck --check-prefix=CHECK-RV64 /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/RISCV/rvv-intrinsics/vluxseg.c
60,050 msx64 debian > Clang.CodeGen/RISCV/rvv-intrinsics::vsuxseg.c
Script: -- : 'RUN: at line 3'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/15.0.0/include -nostdsysteminc -triple riscv64 -target-feature +f -target-feature +d -target-feature +zfh -target-feature +v -disable-O0-optnone -emit-llvm /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/RISCV/rvv-intrinsics/vsuxseg.c -o - | /var/lib/buildkite-agent/builds/llvm-project/build/bin/opt -S -mem2reg | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck --check-prefix=CHECK-RV64 /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/RISCV/rvv-intrinsics/vsuxseg.c
View Full Test Results (10 Failed)

Event Timeline

Jim requested review of this revision.Feb 11 2022, 1:18 AM
Jim created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 11 2022, 1:18 AM

Can you add a test for this change?

Jim added a comment.Feb 14 2022, 10:22 PM

Can you add a test for this change?

The crash only triggered target is given (specify -mtriple=riscv64) and BasicTTIImplBase::getCommonMaskedMemoryOpCost would be called.
If no target given, NoTTIImpl is used as TTI, and TargetTransformInfoImpl.h::getMemoryOpCost simply returns cost 1 without any crash.

I don't have plan to implement hook getMaskedMemoryOpCost for RISCV.
Do you have any suggest for this kind situation?

Can you add a test for this change?

The crash only triggered target is given (specify -mtriple=riscv64) and BasicTTIImplBase::getCommonMaskedMemoryOpCost would be called.
If no target given, NoTTIImpl is used as TTI, and TargetTransformInfoImpl.h::getMemoryOpCost simply returns cost 1 without any crash.

I don't have plan to implement hook getMaskedMemoryOpCost for RISCV.
Do you have any suggest for this kind situation?

This function assumes fixed-width vectors and cannot be used for scalable vectors. I'm not sure what returning 'Invalid' here would really fix, other than the compiler not crashing for a use-case that should not have occurred in the first place, because an overloaded cost function should have been implemented. Whether the compiler falls into the assert from cast<FixedVectorType>, or whether it returns an Invalid cost, in either case you'll need to implement a memory-op-cost function for your target. And because you'll need to implement a cost-function anyway, returning Invalid doesn't really make a difference, because then this code will never be hit. Otherwise, you would have been able to write a test-case for it.

This function assumes fixed-width vectors and cannot be used for scalable vectors. I'm not sure what returning 'Invalid' here would really fix, other than the compiler not crashing for a use-case that should not have occurred in the first place, because an overloaded cost function should have been implemented. Whether the compiler falls into the assert from cast<FixedVectorType>, or whether it returns an Invalid cost, in either case you'll need to implement a memory-op-cost function for your target. And because you'll need to implement a cost-function anyway, returning Invalid doesn't really make a difference, because then this code will never be hit. Otherwise, you would have been able to write a test-case for it.

Many salable vectors use this function and crash . Maybe D121677 is a test-case. GetMaskedMemoryOpCost and getGatherScatterOpCost only return getCommonMaskedMemoryOpCost , so each scalable vector call to these functions will crash. However, getMaskedMemoryOpCost and getGatherScatterOpCost can be implemented by each target, not implemented will it crashes. There seems to be a problem.

Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2022, 6:57 PM