[PATCH] D59062: [GlobalISel][AArch64] Always fall back on aarch64.neon.addp.*

[Amara Emerson via Phabricator via llvm-commits]

aemerson added inline comments.


================
Comment at: lib/Target/AArch64/AArch64LegalizerInfo.cpp:518-527
+  // HACK: Don't allow faddp/addp for now. We don't pass down the type info
+  // necessary to get this right today.
+  //
+  // It looks like addp/faddp is the only intrinsic that's impacted by this.
+  // All other intrinsics fully describe the required types in their names.
+  //
+  // (See: https://bugs.llvm.org/show_bug.cgi?id=40968)
----------------
kristof.beyls wrote:
> Over time, more intrinsics get added.
> So, making an exception based on a specific intrinsic name here seems like it's a time bomb waiting to go off when new intrinsics get added which have the same properties resulting in a silent codegen faults.
> Wouldn't it be possible to check for the properties of the intrinsics that result in incorrect code generation and check on those properties rather than explicitly blacklist based on name?
You're right, this is not a solution that fixes the real issue. It's a workaround that we plan to be in place until we settle on a way to fix this in the general case.

Unfortunately, barring disallowing all potentially int/fp type overloaded intrinsics during translation, we don't have a good way to detect these problematic ones automatically. This issue happens when no other attribute of the intrinsic call disambiguates the selected opcode, so element size etc isn't enough. Knowing that isn't generally possible unless we analyze all the selection patterns.

If you like, we could expand this fallback to something more general, up until falling back to all neon intrinsics. However I think that's probably unnecessary.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59062/new/

https://reviews.llvm.org/D59062