This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Split the neon.addp intrinsic into integer and fp variants
ClosedPublic

Authored by aemerson on Mar 21 2019, 10:55 AM.

Details

Summary

This is the result of discussions on the list about how to deal with intrinsics which require codegen to disambiguate them via only the integer/fp overloads. It causes problems for GlobalISel as some of that information is lost during translation, while with other operations like IR instructions the information is encoded into the instruction opcode.

This patch changes clang to emit the new faddp intrinsic if the vector operands to the builtin have FP element types. LLVM IR AutoUpgrade has been taught to upgrade existing calls to aarch64.neon.addp with fp vector arguments, and we remove the workarounds introduced for GlobalISel in r355865.

This is a more permanent solution to PR40968.

Diff Detail

Repository
rL LLVM

Event Timeline

aemerson created this revision.Mar 21 2019, 10:55 AM
aemerson updated this revision to Diff 191745.Mar 21 2019, 10:59 AM

Minor test tweak.

I've put up a langref change as a separate review: D59657

efriedma added inline comments.
llvm/lib/IR/AutoUpgrade.cpp
574 ↗(On Diff #191745)

This code is weird... you're computing the types in two different ways. Also, missing a check for F->arg_size() (so we don't crash on invalid IR).

aemerson marked an inline comment as done.Mar 21 2019, 1:00 PM
aemerson added inline comments.
llvm/lib/IR/AutoUpgrade.cpp
574 ↗(On Diff #191745)

I'll consolidate the logic, but none of the other code here checks for IR validity. By the time we reach here the IR should be valid, we're just translating it to a newer version. I can put an assert anyway.

The IR at this

llvm/lib/IR/AutoUpgrade.cpp
574 ↗(On Diff #191745)

The IR during autoupgrade should be loosely "valid", to the point of passing the asm/bitcode parser, but we don't check the signature of intrinsics until later, so someone could write declare <4 x float> @llvm.aarch64.neon.addp.v4f32().

aemerson updated this revision to Diff 191774.Mar 21 2019, 1:48 PM

Simplify logic and don't try to upgrade if IR is invalid.

This revision is now accepted and ready to land.Mar 21 2019, 2:04 PM
This revision was automatically updated to reflect the committed changes.
llvm/trunk/lib/Target/AArch64/AArch64LegalizerInfo.cpp