[PATCH] D50065: [LLD] Only increase LMARegion if different from MemRegion

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


grimar added a comment.

Overall looks good to me. Comments are below.
(I would rename the test from `at8.s` to `at6.s` though, since we do not have `at6.s` yet.)



================
Comment at: test/ELF/linkerscript/at8.test:10
+
+REGION_ALIAS("RAM2", RAM)
+
----------------
We usually try to reduce the test cases.
This alias seems to be excessive here, so I would remove.
Otherwise, it looks like having an alias is important, though it is not.


================
Comment at: test/ELF/linkerscript/at8.test:17
+
+# CHECK: .text             PROGBITS        0000000020000000 001000
+# CHECK: .sec             PROGBITS        0000000020000001 001001
----------------
Please add a brief comment describing what are you checking in this test case.

I would also add a header to show what is 0000000020000000 and 001000.
(like you have for program headers below).


================
Comment at: test/ELF/linkerscript/at8.test:21
+# CHECK: Program Headers:
+# CHECK-NOT: LOAD
+
----------------
I think this line does not make sense. All LOADs are always listed after the header:
`Type  Offset   VirtAddr           PhysAddr`
and you have `CHECK-NEXT` after it.



Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D50065





More information about the llvm-commits mailing list