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

Francesco Petrogalli via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 12 12:05:54 PDT 2019


fpetrogalli marked 34 inline comments as done.
fpetrogalli added a comment.

Thank you @ABataev.



================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9698
+/// as defined by `LS(P)` in 3.2.1 of the AAVFABI.
+unsigned LaneSize(QualType QT, ParamKindTy Kind, ASTContext &C) {
+  if (MTV(QT, Kind) && QT->isPointerType()) {
----------------
ABataev wrote:
> ABataev wrote:
> > 1. Function names should be verb phrases (as they represent actions), and command-like function should be imperative. The name should be camel case, and start with a lower case letter (e.g. openFile() or isFoo()).
> > 2. Mark function as static
> Size must be in bits or in bytes?
I am using bytes, but it doesn't really matter, it is not required by the ABI. Do you have a preference?


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9832
+                                    const unsigned VecRegSize,
+                                    llvm::Function *Fn = nullptr) {
+  // Get basic data for building the vector signature.
----------------
ABataev wrote:
> Do you really want to allow `nullptr` here? I think in this case all this stuff is useless
Yep, I have no idea why I added a nullptr default here...


================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:10033
+                                         MangledName, 's', 128, Fn);
+        if (CGM.getTarget().hasFeature("neon"))
+          emitAArch64DeclareSimdFunction(CGM, FD, VLEN, ParamAttrs, State,
----------------
ABataev wrote:
> else if. Or both features may exist at the same time?
SVE implies AdvSIMD, so we need to generate the SIMD names too.


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

https://reviews.llvm.org/D60583





More information about the llvm-commits mailing list