Page MenuHomePhabricator

[Legalizer] Workaround for scalarizing unary strict-fp ops
AbandonedPublic

Authored by qiucf on Mar 29 2020, 9:52 PM.

Details

Summary

This was found when working on D64193. On PowerPC if we turn off VSX feature, we'll get an ICE complaining null table entry in ScalarizeVecRes_StrictFPOp. Here we use the same workaround in ScalarizeVecRes_UnaryOp and ScalarizeVecRes_SETCC.

Diff Detail

Unit TestsFailed

TimeTest
4,720 mslldb-api.functionalities/thread/concurrent_events::Unknown Unit Message ("")
Script: -- /usr/bin/python /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/lldb/test/API/dotest.py --arch x86_64 -s /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/lldb-test-traces -S nm -u CXXFLAGS -u CFLAGS --env ARCHIVER=/usr/bin/ar --env OBJCOPY=/usr/bin/objcopy --env LLVM_LIBS_DIR=/mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/./lib --build-dir /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/lldb-test-build.noindex --lldb-module-cache-dir /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/./bin/lldb --compiler /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/./bin/clang --dsymutil /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/./bin/dsymutil --filecheck /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/./bin/FileCheck --lldb-libs-dir /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/./lib /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/lldb/test/API/functionalities/thread/concurrent_events -p TestConcurrentBreakpointsDelayedBreakpointOneWatchpoint.py
140 mslldb-unit.Host/_/HostTests::Unknown Unit Message ("")
Note: Google Test filter = ConnectionFileDescriptorTest.TCPGetURIv6 [==========] Running 1 test from 1 test case. [----------] Global test environment set-up.

Event Timeline

qiucf created this revision.Mar 29 2020, 9:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2020, 9:52 PM
qiucf edited the summary of this revision. (Show Details)Mar 29 2020, 10:39 PM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
230

Can this if-else be rolled into the generic loop below? Or is there a reason that's not ok?

qiucf planned changes to this revision.Apr 1 2020, 9:45 AM
qiucf marked an inline comment as done.

Add tests based on revision D64193.

llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
230

That's because every strict-fp operation will have extra operand at first position for chain. While ScalarizeVecRes_UnaryOp just takes the first operand.

craig.topper added inline comments.
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
230

The generic loop below skips the first operand so we should just be able to put this getTypeAction and EXTRACT_VECTOR_ELT handling into that loop.

qiucf updated this revision to Diff 254478.Apr 2 2020, 4:11 AM
qiucf marked 3 inline comments as done.

Address some comments

qiucf added inline comments.Apr 2 2020, 4:16 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
230

Thanks for reminder! I misunderstood the comment.

kpn added a subscriber: kpn.Apr 6 2020, 12:49 PM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
230

Should we copy the workaround comment from ScalarizeVecRes_UnaryOp(...) to this change? The workaround is pretty confusing to me, but I'm not an expert here.

I don't think this is needed for PowerPC after 1a493b0fa556a07c728862c3c3f70bfd8683bef0.

We really should be scalarizing single element vectors and after that change, we will.

RKSimon resigned from this revision.May 29 2020, 4:23 AM
qiucf abandoned this revision.Jun 1 2020, 7:30 PM

@nemanjai : I don't think this is needed for PowerPC after 1a493b0fa556a07c728862c3c3f70bfd8683bef0.

It works. Thanks!