[PATCH] D131547: [Clang][AArch64] Use generic extract/insert vector for svget/svset/svcreate tuples

Thu Aug 18 06:53:33 PDT 2022

sdesmalen added a comment.

Thanks for addressing the comments @CarolineConcatto!

In D131547#3731310 <https://reviews.llvm.org/D131547#3731310>, @dmgreen wrote:

>> Is there a formal requirement that LLVM must remain backward compatible with older LLVM IR (beyond the target-independent parts)?
>
> We have always done it in the past, and I don't see a good reason to change. This change is essentially for llvm 16, so we are talking about any bitcode between when SVE was added and that release. It is hard to tell how people will use bitcode up to that point and if they will expect it to continue working going forward. I think it's simpler to just add the upgrade code, than to try and argue that it is unneeded. But the upgrade code is really needed in D131548 <https://reviews.llvm.org/D131548> (and D131687 <https://reviews.llvm.org/D131687>) where the old intrinsics are being removed.

It seems that the LLVM Developer Policy <https://llvm.org/docs/DeveloperPolicy.html?highlight=backwards#ir-backwards-compatibility> provides better guidance than that.

> Newer releases can ignore features from older releases, but they cannot miscompile them

Removing the intrinsics but not auto-upgrading them would mean that older IR would miscompile (the call to the intrinsic would become an actual function call). This suggests there is no freedom of choice here and we must use AutoUpgrade.

================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:9099
+                                             ArrayRef<Value *> Ops) {
+
+  assert(TypeFlags.isTupleCreate() && "Expects TypleFlag isTupleCreate");
----------------
nit: redundant newline, please remove.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131547