This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Enable Rounding Double Multiply Add/Subtract instructions on Falkor.
ClosedPublic

Authored by mcrosier on Jan 13 2017, 8:14 AM.

Details

Summary

Falkor only partially implements the ARMv8.1a extensions, so this patch refactors the support for the SQRDML[A|S]H instruction into a separate feature.

Chad

Diff Detail

Repository
rL LLVM

Event Timeline

mcrosier updated this revision to Diff 84309.Jan 13 2017, 8:14 AM
mcrosier retitled this revision from to [AArch64] Enable Rounding Double Multiply Add/Subtract instructions on Falkor..
mcrosier updated this object.
mcrosier added reviewers: t.p.northover, jmolloy, aadg, gberry, ab.
mcrosier added a subscriber: llvm-commits.
ab accepted this revision.Jan 13 2017, 11:10 AM
ab edited edge metadata.

LGTM

test/CodeGen/AArch64/arm64-neon-v8.1a.ll
2 ↗(On Diff #84309)

Maybe add --check-prefix=CHECK-RDM to the v8.1a line, and remove the rdm CHECK-V81a lines?

This revision is now accepted and ready to land.Jan 13 2017, 11:10 AM
mcrosier added inline comments.Jan 13 2017, 12:32 PM
test/CodeGen/AArch64/arm64-neon-v8.1a.ll
2 ↗(On Diff #84309)

I'm not sure I follow, Ahmed. Are you suggesting that I not add an additional RUN line and replace

; RUN: llc < %s -verify-machineinstrs -mtriple=arm64-eabi -mattr=+v8.1a -aarch64-neon-syntax=generic | FileCheck %s --check-prefix=CHECK-V81a

with

; RUN: llc < %s -verify-machineinstrs -mtriple=arm64-eabi -mattr=+rdm -aarch64-neon-syntax=generic | FileCheck %s --check-prefix=CHECK-RDM

and update the CHECK-V81a directives to CHECK-RDM?

I'm happy to do that, I just want to confirm that is what you were suggesting.

ab added inline comments.Jan 13 2017, 12:41 PM
test/CodeGen/AArch64/arm64-neon-v8.1a.ll
2 ↗(On Diff #84309)

Oh sorry: I was thinking you'd add the RUN line:

; RUN: llc < %s -verify-machineinstrs -mtriple=arm64-eabi -mattr=+rdm -aarch64-neon-syntax=generic | FileCheck %s --check-prefix=CHECK-RDM
; RUN: llc < %s -verify-machineinstrs -mtriple=arm64-eabi -mattr=+v8.1a -aarch64-neon-syntax=generic | FileCheck %s --check-prefix=CHECK-V81a --check-prefix=CHECK-RDM

Which lets you avoid duplicating the CHECK-RDM lines; e.g., instead of:

    %prod = call <4 x i16> @llvm.aarch64.neon.sqrdmulh.v4i16(<4 x i16> %mhs,  <4 x i16> %rhs)
    %retval =  call <4 x i16> @llvm.aarch64.neon.sqadd.v4i16(<4 x i16> %acc,  <4 x i16> %prod)
 ; CHECK-V8a:        sqrdmulh    v1.4h, v1.4h, v2.4h
+; CHECK-RDM:        sqrdmlah    v0.4h, v1.4h, v2.4h
 ; CHECK-V81a:       sqrdmlah    v0.4h, v1.4h, v2.4h
 ; CHECK-V81a-apple: sqrdmlah.4h v0,    v1,    v2
    ret <4 x i16> %retval

you'd do:

    %prod = call <4 x i16> @llvm.aarch64.neon.sqrdmulh.v4i16(<4 x i16> %mhs,  <4 x i16> %rhs)
    %retval =  call <4 x i16> @llvm.aarch64.neon.sqadd.v4i16(<4 x i16> %acc,  <4 x i16> %prod)
 ; CHECK-V8a:        sqrdmulh    v1.4h, v1.4h, v2.4h
-; CHECK-V81a:       sqrdmlah    v0.4h, v1.4h, v2.4h
+; CHECK-RDM:        sqrdmlah    v0.4h, v1.4h, v2.4h
 ; CHECK-V81a-apple: sqrdmlah.4h v0,    v1,    v2
    ret <4 x i16> %retval

Not a big deal either way, really.

This revision was automatically updated to reflect the committed changes.

I settled on adding a new RUN line, but used the CHECK-V81a prefix. I believe this was in the same vein as your original suggestion, Ahmed, but minimized code changes while still providing the necessary testing coverage.