[PATCH] D49993: [LLD][ELF][ARM] Implement support for Tag_ABI_VFP_args

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 31 00:45:55 PDT 2018


grimar added a comment.

Looks good to me too. Minor comments below.



================
Comment at: ELF/InputFiles.cpp:501
+static void updateARMVFPArgs(const ARMAttributeParser &Attributes,
+                             const elf::InputFile *F) {
+  if (!Attributes.hasAttribute(ARMBuildAttrs::ABI_VFP_args))
----------------
You do not need the namespace name here (`elf::`) I think.


================
Comment at: ELF/InputFiles.cpp:520
+  case 2:
+    // toolchain specific convention that conforms to neither AAPCS variant.
+    Arg = ARMVFPArgKind::ToolChain;
----------------
toolchain -> Toolchain 


================
Comment at: ELF/InputFiles.cpp:526
+  default:
+    return;
+  }
----------------
Should default (it's unknown, right?) value trigger an error/warning/unreachable instead of return?


================
Comment at: test/ELF/Inputs/arm-vfp-arg-base.s:11
+	.eabi_attribute 18, 4
+        .eabi_attribute 28, 0
+
----------------
I would add a comment, just like in other inputs you have.


https://reviews.llvm.org/D49993





More information about the llvm-commits mailing list