Page MenuHomePhabricator

[AArch64][SVE] Implement extractelement of i1 vectors.
Needs ReviewPublic

Authored by efriedma on Sep 14 2020, 4:18 PM.

Details

Summary

The implementation just extends the vector to a larger element type, and extracts from that. Not fancy, but generates reasonable code.

While I'm here, fix warning from computeKnown bits.

Diff Detail

Unit TestsFailed

TimeTest
430 mslinux > HWAddressSanitizer-x86_64.TestCases::sizes.cpp
Script: -- : 'RUN: at line 3'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang --driver-mode=g++ -m64 -gline-tables-only -fsanitize=hwaddress -fuse-ld=lld -mcmodel=large -mllvm -hwasan-globals -mllvm -hwasan-use-short-granules -mllvm -hwasan-instrument-landing-pads=0 -mllvm -hwasan-instrument-personality-functions /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/hwasan/TestCases/sizes.cpp -nostdlib++ -lstdc++ -o /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/hwasan/X86_64/TestCases/Output/sizes.cpp.tmp
90 mslinux > Polly.ScopInfo/NonAffine::non-affine-loop-condition-dependent-access_3.ll
Script: -- : 'RUN: at line 1'; opt -polly-process-unprofitable -polly-remarks-minimal -polly-use-llvm-names -polly-import-jscop-dir=/mnt/disks/ssd0/agent/llvm-project/polly/test/ScopInfo/NonAffine -polly-codegen-verify -basic-aa -polly-scops -polly-allow-nonaffine-branches -polly-allow-nonaffine-loops=false -analyze < /mnt/disks/ssd0/agent/llvm-project/polly/test/ScopInfo/NonAffine/non-affine-loop-condition-dependent-access_3.ll | FileCheck /mnt/disks/ssd0/agent/llvm-project/polly/test/ScopInfo/NonAffine/non-affine-loop-condition-dependent-access_3.ll --check-prefix=INNERMOST

Event Timeline

efriedma created this revision.Sep 14 2020, 4:18 PM
efriedma requested review of this revision.Sep 14 2020, 4:18 PM
paulwalker-arm added inline comments.Sep 15 2020, 3:16 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
9290–9296

This is a common route when expanding i1 based operations so downstream we did it in common code, for example:

setOperationAction(ISD::EXTRACT_VECTOR_ELT, MVT::nxv2i1, Promote);
AddPromotedToType(ISD::EXTRACT_VECTOR_ELT, MVT::nxv2i1, MVT::nxv2i64);

and updated SelectionDAGLegalize::PromoteNode accordingly. Is this route something we can do upstream? instead of having custom lowering.

efriedma added inline comments.Sep 15 2020, 2:19 PM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
9290–9296

The problem is that "promote" is currently interpreted to mean "bitcast the vector operands" by vector legalization, not integer promotion, so it would be confusing to use it like this. Maybe worth messing with LegalizeAction to distingush between the two kinds of "promotion", though.

paulwalker-arm added inline comments.Sep 16 2020, 2:55 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
9290–9296

Within AArch64ISelLowering.cpp I can see:

setOperationAction(ISD::FADD, MVT::v4f16, Promote);
AddPromotedToType(ISD::FADD, MVT::v4f16, MVT::v4f32);

With SelectionDAGLegalize::PromoteNode applying the appropriate extension. I can see similar usages like this for other targets. To me it looks like "Promote" can mean promotion or bitcast, with nodes having support for whichever has been required in the past. Based on this it seems safe to extend PromoteNode's handling of EXTRACT_VECTOR_ELT to include real promotion.

efriedma updated this revision to Diff 299476.Tue, Oct 20, 2:36 PM
efriedma edited the summary of this revision. (Show Details)

Rebase and clang-format.

I looked into handling this in LegalizeVectorOps, but that doesn't really work out: we actually legalize EXTRACT_VECTOR_ELT nodes in LegalizeDAG. And I don't want to do this in LegalizeDAG: LegalizeDAG can't handle arbitrary illegal vector operations.

There might be some way to sort out this mess, but it isn't trivial.