[ConstantFold] Fold bitcasts on scalable vector splats.
Needs ReviewPublic
Actions

Authored by sdesmalen on May 12 2022, 12:32 AM.

Details

Reviewers

david-arm
kmclaughlin
craig.topper
frasercrmck

Summary

This patch also contains changes to InstCombine to avoid a regression for
scalable-const-fp-splat.ll which otherwise would no longer be folded
because the expression is not an explicit ConstExpr any more, but a folded
splat instead.

This patch additionally fixes

https://github.com/llvm/llvm-project/issues/55348.

where one of the operands to be sunk was a ConstExpr. Because the m_pattern()
functions also match ConstExpr's, the cast<Instruction> following the match
led to an assertion failure. Because we now fold the ConstExpr, there is no
longer a way to represent a sext/zext as a ConstExpr and the patterns no
longer match.

Diff Detail

Repository: rG LLVM Github Monorepo

Unit TestsFailed
View All

	Time	Test
	560 ms	x64 debian > HWAddressSanitizer-x86_64.TestCases::hwasan_symbolize.cpp Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -m64 -gline-tables-only -fsanitize=hwaddress -fuse-ld=lld -fsanitize-hwaddress-experimental-aliasing -mllvm -hwasan-generate-tags-with-calls=1 -mllvm -hwasan-globals -mllvm -hwasan-use-short-granules -mllvm -hwasan-instrument-landing-pads=0 -mllvm -hwasan-instrument-personality-functions -Wl,--build-id -g /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/hwasan/TestCases/hwasan_symbolize.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/hwasan/X86_64/TestCases/Output/hwasan_symbolize.cpp.tmp
	1,010 ms	x64 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

sdesmalen created this revision.May 12 2022, 12:32 AM

Herald added a project: Restricted Project. · View Herald TranscriptMay 12 2022, 12:32 AM

Herald added subscribers: StephenFan, hiraditya. · View Herald Transcript

sdesmalen requested review of this revision.May 12 2022, 12:32 AM

Herald added a project: Restricted Project. · View Herald TranscriptMay 12 2022, 12:32 AM

Herald added subscribers: llvm-commits, alextsao1999. · View Herald Transcript

Harbormaster completed remote builds in B164049: Diff 428865.May 12 2022, 1:38 AM

From what I gather about the bug report, RISCV doesn't suffer from it as we always dyn_cast to Instruction. Is that right? Just wondering if we should add a similar test - I don't think we have constant expr coverage in sink-splat-operands.ll.

llvm/lib/IR/ConstantFold.cpp
406	`DestVecTy` == `DestVTy`, no?
409	`cast<VectorType>(DestTy)` == `DestVTy`.
llvm/test/Transforms/InstCombine/addrspacecast.ll
206	This one strikes me as peculiar. Is it all less canonical than it was before?

Addressed comments.
Reverted the fixed-width change (leading to different addrspacecast constant)

In D125449#3508487, @frasercrmck wrote:

From what I gather about the bug report, RISCV doesn't suffer from it as we always dyn_cast to Instruction. Is that right? Just wondering if we should add a similar test - I don't think we have constant expr coverage in sink-splat-operands.ll.

That's what I did initially, but I couldn't write a test with a ZExt/SExt as ConstExpr that wouldn't fold, so that would mean I'd rewrite the code without being able to test it.

llvm/lib/IR/ConstantFold.cpp
406	You're right, I moved this code around a few times and didn't spot this.
llvm/test/Transforms/InstCombine/addrspacecast.ll
206	You're right, that's probably not desired, because addrspacecast's don't fold any further. I've reverted that change now.