This is an archive of the discontinued LLVM Phabricator instance.

[ARM][MVE] Add intrinsics for immediate shifts.
ClosedPublic

Authored by simon_tatham on Dec 5 2019, 6:24 AM.

Details

Summary

This adds the family of vshlq_n and vshrq_n ACLE intrinsics, which
shift every lane of a vector left or right by a compile-time
immediate. They mostly work by expanding to the IR shl, lshr and
ashr operations, with their second operand being a vector splat of
the immediate.

There's a fiddly special case, though. ACLE specifies that the
immediate in vshrq_n can take values up to and including the bit
size of the vector lane. But LLVM IR thinks that shifting right by the
full size of the lane is UB, and feels free to replace the lshr with
an undef half way through the optimization pipeline. Hence, to keep
this legal in source code, I have to detect it at codegen time.
Logical (unsigned) right shifts by the element size are handled by
simply emitting the zero vector; arithmetic ones are converted into a
shift of one bit less, which will always give the same output.

In order to do that check, I also had to enhance the tablegen
MveEmitter so that it can cope with converting a builtin function's
operand into a bare integer to pass to a code-generating subfunction.
Previously the only bare integers it knew how to handle were flags
generated from within arm_mve.td.

Diff Detail

Event Timeline

simon_tatham created this revision.Dec 5 2019, 6:24 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptDec 5 2019, 6:24 AM
This revision is now accepted and ready to land.Dec 6 2019, 9:54 AM
This revision was automatically updated to reflect the committed changes.
simon_tatham reopened this revision.Dec 10 2019, 2:24 AM

Reopening to review a revised version of this patch. It was reverted yesterday because of a test failure in release builds, which looks like the result of a warning fix that moved a side effect into an assert statement.

This revision is now accepted and ready to land.Dec 10 2019, 2:24 AM

Changes from previous version:

  • minor cleanup: removed check of hasIntegerConstantValue in IRBuilderResult::more_prerequisites, which was causing the generated codegen to perofrm a pointless call to EmitScalarExpr whose result was thrown away.
  • did not incorporate the -Wunused-variable change from rGff4dceef9201c5ae which moved a side-effecting function call into an assert statement. Instead fixed it with a (void)IsConst like all the other call sites.

@hokein , @rdhindsa , @echristo : you all pointed out test failures in the previous version. Any problems I haven't spotted with this one?

dmgreen accepted this revision.Dec 11 2019, 1:28 AM

One of the advantages to smaller patches I guess :)

It's probably difficult to tell if this will cause problems again without trying it and see if any of the buildbots complain. Lets give it a try and see. Just keep an eye on them, we annoyingly don't get any of the failure emails that we should.

clang/utils/TableGen/MveEmitter.cpp
674

This Sep isn't needed any more?

This revision was automatically updated to reflect the committed changes.