[PATCH] D45336: Apply accumulator to fadd/fmul experimental vector reductions (PR36734)

[Amara Emerson via Phabricator via llvm-commits]

aemerson added a comment.

In https://reviews.llvm.org/D45336#1060178, @RKSimon wrote:

> In https://reviews.llvm.org/D45336#1060080, @aemerson wrote:
>
> > The other issue is that while these intrinsics were experimental (it was on my todo list for later this year to change that), AArch64 has been using them with their original intended semantics for a while now. If we change that, IR generated from a released compiler will be miscompiled since it now becomes legal to fold away undef accumulator reductions to undef, unless we do some IR auto-upgrading based on the bitcode version.
>
>
> If they are flagged as experimental surely we can't be held resposible for changes in behaviour?


AArch64 was the test guinea pig for this new representation, and it's proved to be a smooth transition. If you change the semantics now, even if the intrinsics are still experimental in name, IR from LLVM 6.0 may be silently miscompiled if someone implements a valid optimization based on your new proposal. That's a fact, and I don't think this patch review is the right place to discuss that if you want to do this, I suggest you send a new RFC or revive the old one.


Repository:
  rL LLVM

https://reviews.llvm.org/D45336