Page MenuHomePhabricator

[AArch64][SVE] Improve sve.convert.to.svbool lowering
ClosedPublic

Authored by peterwaller-arm on Apr 29 2021, 9:28 AM.

Details

Summary

The sve.convert.to.svbool lowering has the effect of widening a logical
<M x i1> vector representing lanes into a physical <16 x i1> vector
representing bits in a predicate register.

In general, if converting to svbool, the contents of lanes in the
physical register might not be known. For sve.convert.to.svbool the new
lanes are specified to be zeroed, requiring 'and' instructions to mask
off the new lanes. For lanes coming from a ptrue or a comparison,
however, they are known to be zero.

CodeGen Before:

ptrue p0.s, vl16
ptrue p1.s
ptrue p2.b
and   p0.b, p2/z, p0.b, p1.b
ret

After:

ptrue	p0.s, vl16
ret

Diff Detail

Unit TestsFailed

TimeTest
80 msx64 debian > Clang.Driver::debug-pass-structure.c
Script: -- : 'RUN: at line 2'; /mnt/disks/ssd0/agent/llvm-project/build/bin/clang -fexperimental-new-pass-manager -fdebug-pass-structure -O3 -S -emit-llvm /mnt/disks/ssd0/agent/llvm-project/clang/test/Driver/debug-pass-structure.c -o /dev/null 2>&1 | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/clang/test/Driver/debug-pass-structure.c --check-prefix=NEWPM

Event Timeline

peterwaller-arm requested review of this revision.Apr 29 2021, 9:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 29 2021, 9:28 AM
Matt added a subscriber: Matt.Apr 29 2021, 12:38 PM
paulwalker-arm added inline comments.May 2 2021, 3:07 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
3684

This is used five times within the function so perhaps worth having SDValue InOp?

3701

Can you use getConstantOperandVal here?

llvm/test/CodeGen/AArch64/sve-intrinsics-reinterpret.ll
87

If this required? Same comment for reinterpret_cmpgt.

88

I think you can be more precise here as we are expecting ptrue p0.h

99

Is this required? I would have thought we can check for the precise output. If you pass the mask as a parameter I think we'd effectively have:

CHECK: cmpgt p0/z, z0.h, z1.h
CHECK-NEXT: ret

In fact there's a good shout here for just using llvm/utils/update_llc_test_checks.py to generate the check lines for this file.

  • Address review comments.
  • Use update_llc_test_checks to generate tests.
peterwaller-arm marked 5 inline comments as done.May 6 2021, 5:46 AM
peterwaller-arm edited the summary of this revision. (Show Details)

Push updates missed out due to an error of mine on the last run.

  • Address review comments.
  • Use update_llc_test_checks to generate tests.

Fix wrong indentation.

paulwalker-arm accepted this revision.May 11 2021, 9:47 AM
paulwalker-arm added inline comments.
llvm/test/CodeGen/AArch64/sve-intrinsics-reinterpret.ll
86

Personally I see this as a function comment rather than belonging with the CHECK lines.

103

You can simplify the test by passing cmpgt's predicate as a function parameter.

This revision is now accepted and ready to land.May 11 2021, 9:47 AM

Update for review comments.

This revision was landed with ongoing or failed builds.May 12 2021, 2:58 AM
This revision was automatically updated to reflect the committed changes.