[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:9861
+  // limits.
+  if ((ISA == 's') && UserVLEN != 0) {
+    if ((UserVLEN * WDS > 2048) || (UserVLEN * WDS % 128 != 0)) {
----------------
ABataev wrote:
> Remove extra parens
Parens around `ISA == 's'` still in here


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9684
+static bool getAArch64PBV(QualType QT) {
+  if (QT->isFloatingType())
+    return true;
----------------
Use QT.getCanonicalType() here too


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9698
+/// as defined by `LS(P)` in 3.2.1 of the AAVFABI.
+static unsigned getAArch64LS(QualType QT, ParamKindTy Kind, ASTContext &C) {
+  if (getAArch64MTV(QT, Kind) && QT->isPointerType()) {
----------------
Same here, use QT.getCanonicalType() here too


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9715
+getNDSWDS(const FunctionDecl *FD, ArrayRef<ParamAttrTy> ParamAttrs) {
+  QualType RetType = FD->getReturnType();
+
----------------
Use FD->getReturnType().getCanonicalType()


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9728
+  for (unsigned I = 0, E = FD->getNumParams(); I < E; ++I) {
+    QualType QT = FD->getParamDecl(I)->getType();
+    Sizes.push_back(getAArch64LS(QT, ParamAttrs[I].Kind, C));
----------------
.getCanonicalType()


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9845
+        "The clause simdlen(1) has no effect when targeting aarch64.");
+    CGM.getDiags().Report(SLoc, DiagID);
+    return;
----------------
Better to use the location from the original VLEN expression. Currently it points to the function and does not help the user.


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9855
+                                    "power of 2 when targeting Advanced SIMD.");
+    CGM.getDiags().Report(SLoc, DiagID);
+    return;
----------------
Same here, use the location from the VLEN


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9880
+    if (ISA == 's')
+      addAArch64VectorName(UserVLEN, "M", Prefix, ISA, ParSeq, MangledName,
+                           OutputBecomesInput, Fn);
----------------
Enclose into braces, if one substatement in braces, all the substatements must be in braces


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9906
+    if (ISA == 's')
+      addAArch64VectorName("x", "M", Prefix, ISA, ParSeq, MangledName,
+                           OutputBecomesInput, Fn);
----------------
Same, braces arounf substatement


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:10023
           CGM.getTriple().getArch() == llvm::Triple::x86_64)
         emitX86DeclareSimdFunction(FD, Fn, VLENVal, ParamAttrs, State);
+      else if (CGM.getTriple().getArch() == llvm::Triple::aarch64) {
----------------
Enclose into braces


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:10025
+      else if (CGM.getTriple().getArch() == llvm::Triple::aarch64) {
+        unsigned VLEN = VLENVal.getExtValue();
+        StringRef MangledName = Fn->getName();
----------------
Will it work if the VLEN was not specified?


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

https://reviews.llvm.org/D60583