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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

Overall looking good.



================
Comment at: ELF/InputFiles.cpp:497
 
+// To set the EF_ARM_ABI_FLOAT_SOFT or EF_ARM_ABI_FLOAT_HARD flag in the
+// ELF Header we need to look at Tag_ABI_VFP_args to find out how the
----------------
I'd start this comment with "This is ARM only" to make it clear that this is ARM only.


================
Comment at: ELF/InputFiles.cpp:512
+  ARMVFPArgKind Arg;
+  switch (VFPArgs) {
+  case ARMBuildAttrs::BaseAAPCS:
----------------
nit: since you are using this local variable only once, you can inline it.


================
Comment at: ELF/InputFiles.cpp:519
+    break;
+  case 2:
+    // toolchain specific convention that conforms to neither AAPCS variant.
----------------
If we don't have an enum variable for this magic variable, should we add that one to LLVM?


https://reviews.llvm.org/D49993