[PATCH] D39140: LLD/ELF/AMDGPU: Process AMDGPU-specific e_flags

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/Arch/AMDGPU.cpp:47
+    cast<ObjFile<ELF64LE>>(ObjectFiles.front())->getObj().getHeader()->e_flags;
+  for (const auto &F : ObjectFiles) {
+    uint32_t CurrentEFlags =
----------------
t-tye wrote:
> Is it worth avoiding checking the first element since it has already been inspected. For example:
> 
> 
> ```
> for (const auto &F : ArrayRef(ObjectFiles).slice(1)) {
> ```
Please add a comment saying that this loop is to verify that all input files have the same e_flags.

`const auto` -> `InputFile *` as we do not use auto unless its type is apparent.

FirstEFlags -> Ret because we usually prefer short variable names.


================
Comment at: ELF/Arch/AMDGPU.cpp:48
+  for (const auto &F : ObjectFiles) {
+    uint32_t CurrentEFlags =
+      cast<ObjFile<ELF64LE>>(F)->getObj().getHeader()->e_flags;
----------------
Can you just remove this temporary variable and directly use the value?


https://reviews.llvm.org/D39140