[PATCH] D60583: [AArch64] Implement Vector Funtion ABI name mangling.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9686
+
+  if (QT->isFloatingType())
+    return true;
----------------
I would suggest to checks the size of the type according to the AAVFABI and limit it with the values 1,2,4 and 8 explicitly (just in case, to handle future types)


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9701
+static unsigned getAArch64LS(QualType QT, ParamKindTy Kind, ASTContext &C) {
+  if (getAArch64MTV(QT, Kind) && QT.getCanonicalType()->isPointerType()) {
+    QualType PTy = QT->getPointeeType().getCanonicalType();
----------------
There is a second part of the requirement `reference to some type T for which PBV(T) is true,`


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9702
+  if (getAArch64MTV(QT, Kind) && QT.getCanonicalType()->isPointerType()) {
+    QualType PTy = QT->getPointeeType().getCanonicalType();
+    if (getAArch64PBV(PTy))
----------------
`QT.getCanonicalType()->getPointeeType()`


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9883
+                           OutputBecomesInput, Fn);
+    } else if (ISA == 'n') {
+      // Advanced SIMD generates one or two functions, depending on
----------------
I think this must be just `else` and in the substatement it would be good to have an assert `assert(ISA == 'n' && "expected ISA either 's' or 'n'.");`


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9910
+                           OutputBecomesInput, Fn);
+    } else if (ISA == 'n') {
+      // Advanced SIMD, Section 3.3.1 of the AAVFABI, generates one or
----------------
I think this must be just `else` and in the substatement it would be good to have an assert `assert(ISA == 'n' && "expected ISA either 's' or 'n'.");`


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

https://reviews.llvm.org/D60583