Page MenuHomePhabricator

[DAGCombiner][WebAssembly] Combine shuffles of more splat vals
Changes PlannedPublic

Authored by tlively on Fri, Jul 10, 7:10 PM.

Details

Summary

combineShuffleOfSplatVal previously only combined shuffles of splat
values that were themselves shuffles. This patch generalizes the
combine to also combine away shuffles of arbitrary splat values
recognized by SelectionDAG::isSplatValue, as long as doing so does not
create any new undefined lanes.

On the WebAssembly side, this patch also introduces a new custom
combine to remove undefined lanes from splatting
build_vectors. Without this extra combine, the new generic shuffle
combine would be inhibited on interesting cases such as the
shl_abs_add function in simd-shift-complex-splats.ll because it would
expose the undefined lanes.

Depends on D83605.

Diff Detail

Unit TestsFailed

TimeTest
460 mslinux > SanitizerCommon-asan-x86_64-Linux.Linux::Unknown Unit Message ("")
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang --driver-mode=g++ -gline-tables-only -fsanitize=address -m64 -ldl -std=c++11 -O0 -g /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/protoent.cpp -o /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Linux/Output/protoent.cpp.tmp && /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Linux/Output/protoent.cpp.tmp 2>&1 | FileCheck /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/protoent.cpp
300 mslinux > SanitizerCommon-lsan-x86_64-Linux.Linux::Unknown Unit Message ("")
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang --driver-mode=g++ -gline-tables-only -fsanitize=leak -m64 -ldl -std=c++11 -O0 -g /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/protoent.cpp -o /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/sanitizer_common/lsan-x86_64-Linux/Linux/Output/protoent.cpp.tmp && /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/sanitizer_common/lsan-x86_64-Linux/Linux/Output/protoent.cpp.tmp 2>&1 | FileCheck /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/protoent.cpp
380 mslinux > SanitizerCommon-msan-x86_64-Linux.Linux::Unknown Unit Message ("")
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang --driver-mode=g++ -gline-tables-only -fsanitize=memory -m64 -ldl -std=c++11 -O0 -g /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/protoent.cpp -o /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/sanitizer_common/msan-x86_64-Linux/Linux/Output/protoent.cpp.tmp && /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/sanitizer_common/msan-x86_64-Linux/Linux/Output/protoent.cpp.tmp 2>&1 | FileCheck /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/protoent.cpp
420 mslinux > SanitizerCommon-tsan-x86_64-Linux.Linux::Unknown Unit Message ("")
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang --driver-mode=g++ -gline-tables-only -fsanitize=thread -m64 -ldl -std=c++11 -O0 -g /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/protoent.cpp -o /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/sanitizer_common/tsan-x86_64-Linux/Linux/Output/protoent.cpp.tmp && /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/sanitizer_common/tsan-x86_64-Linux/Linux/Output/protoent.cpp.tmp 2>&1 | FileCheck /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/protoent.cpp
340 mslinux > SanitizerCommon-ubsan-x86_64-Linux.Linux::Unknown Unit Message ("")
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang --driver-mode=g++ -gline-tables-only -fsanitize=undefined -m64 -ldl -std=c++11 -O0 -g /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/protoent.cpp -o /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/sanitizer_common/ubsan-x86_64-Linux/Linux/Output/protoent.cpp.tmp && /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/sanitizer_common/ubsan-x86_64-Linux/Linux/Output/protoent.cpp.tmp 2>&1 | FileCheck /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/protoent.cpp

Event Timeline

tlively created this revision.Fri, Jul 10, 7:10 PM
Herald added a project: Restricted Project. · View Herald TranscriptFri, Jul 10, 7:10 PM
tlively updated this revision to Diff 277197.Fri, Jul 10, 7:13 PM
  • Remove TODO
tlively updated this revision to Diff 277207.Fri, Jul 10, 7:30 PM
  • Update another x86 test
srj added a subscriber: srj.Mon, Jul 13, 10:33 AM
aheejin added inline comments.Wed, Jul 15, 2:18 PM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
19841

We should fix this comment too now that we allow non-shuffles in side

19852

The previous version seems to allow undef elements in splats, but this forces all elements to be demanded. Is this OK?

19902

It looks like this method deals with a simpler form in which both inner and outer masks are splats, but it seems this has the same limitation that only shuffles are considered. Can this be improved too? (Not necessarily in this patch though)

llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
1749

The same question as above. The previous version of combineShuffleOfSplatVal seems to allow undef lanes from splats. The previous version calls ShuffleVectorSDNode::isSplat, which calls ShuffleVectorSDNode::isSplatMask, which seems to allow undefs. Also the comments there seem to take undef elements in the splat mask in consideration. Do we need to, or, it safe to change it to disallow it? If we allow undef splats in combineShuffleOfSplatVal, we don't need this separate target combine function, right?

tlively planned changes to this revision.Tue, Jul 21, 2:10 PM