Page MenuHomePhabricator

[VP] Legalize the stride operand for EXPERIMENTAL_VP_STRIDED SDNodes
AcceptedPublic

Authored by loralb on Apr 5 2022, 3:01 AM.

Details

Summary

Add promotion and expansion of integer operands for experimental_vp_strided SelectionDAG nodes; the expansion is actually just a truncation of the stride operand.

Diff Detail

Unit TestsFailed

TimeTest
60,110 msx64 debian > AddressSanitizer-x86_64-linux-dynamic.TestCases::scariness_score_test.cpp
Script: -- : 'RUN: at line 4'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only -m64 -shared-libasan -O0 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/asan/TestCases/scariness_score_test.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/asan/X86_64LinuxDynamicConfig/TestCases/Output/scariness_score_test.cpp.tmp
60,160 msx64 debian > AddressSanitizer-x86_64-linux.TestCases::scariness_score_test.cpp
Script: -- : 'RUN: at line 4'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only -m64 -O0 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/asan/TestCases/scariness_score_test.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/asan/X86_64LinuxConfig/TestCases/Output/scariness_score_test.cpp.tmp
60,090 msx64 debian > LLVM.CodeGen/NVPTX::wmma.py
Script: -- : 'RUN: at line 5'; "/usr/bin/python3.9" /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/NVPTX/wmma.py --ptx=60 --gpu-arch=70 > /var/lib/buildkite-agent/builds/llvm-project/build/test/CodeGen/NVPTX/Output/wmma.py.tmp-ptx60-sm_70.ll
60,040 msx64 debian > libFuzzer.libFuzzer::fuzzer-leak.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/LeakTest.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/fuzzer/X86_64DefaultLinuxConfig/Output/fuzzer-leak.test.tmp-LeakTest

Event Timeline

loralb created this revision.Apr 5 2022, 3:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 5 2022, 3:01 AM
loralb requested review of this revision.Apr 5 2022, 3:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 5 2022, 3:01 AM
craig.topper added inline comments.Apr 6 2022, 8:57 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
5098

Use GetExpandedInteger and assign NewOps[OpNo] to Lo

loralb updated this revision to Diff 421471.Apr 8 2022, 3:10 AM

Changelog:

  • Address review comment
loralb marked an inline comment as done.Apr 8 2022, 3:11 AM

Looks good to me but I'd defer to @craig.topper in case I've missed something.

llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
2304

Would this be at all nicer if it was a single assert? assert((opc == load && opno == 3) || (opno == store && opno == 4))? Having explicit if/else just filled with asserts is a little odd to me.

craig.topper added inline comments.May 10 2022, 9:37 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
2304

+1

5091

Probably worth a comment calling out that we drop the upper half.

loralb updated this revision to Diff 431401.May 23 2022, 9:28 AM

Changelog:

  • Address review comments

Is it possible to test this?

loralb added a comment.Thu, Jun 9, 6:44 AM

Is it possible to test this?

I am testing this in D121113, which indeed depends on this patch.
In order to move the tests here, I would have to duplicate the ones in D121113 both for riscv32 and riscv64 and add that revision as parent of this one.
Which approach would you suggest?

craig.topper accepted this revision.Thu, Jun 9, 8:21 AM

Is it possible to test this?

I am testing this in D121113, which indeed depends on this patch.
In order to move the tests here, I would have to duplicate the ones in D121113 both for riscv32 and riscv64 and add that revision as parent of this one.
Which approach would you suggest?

Ok, we can leave them in D121113.

LGTM

This revision is now accepted and ready to land.Thu, Jun 9, 8:21 AM