[PATCH] D81986: [ELF] Refine LMA offset propagation rule in D76395

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

Sounds reasonable to me.



================
Comment at: lld/ELF/LinkerScript.cpp:854
 
+  bool sameMemRegion = ctx->memRegion == sec->memRegion;
   bool prevLMARegionIsDefault = ctx->lmaRegion == nullptr;
----------------
I know we did not do it in LLD before. But what do you think about starting using
the `const` for variables that are not suppored to change?

```
const bool sameMemRegion = ...
```



================
Comment at: lld/test/ELF/linkerscript/lma-offset2.s:8
+# RUN: llvm-readelf -l %t | FileCheck %s
+
+## GNU ld places .text and .bss in the same RWX PT_LOAD.
----------------
Perhaps a description about what this test case does wouldn't hurt.


================
Comment at: lld/test/ELF/linkerscript/lma-offset2.s:17
+  DDR : o = 0xfb00000, l = 185M
+  TCM : o = 0x1f400000, l = 128K
+}
----------------
I'd rename "DDR", "TCM" to something more clear like "TEXT", "DATA"


================
Comment at: lld/test/ELF/linkerscript/lma-offset2.s:22
+  .text : { *(.text) } > DDR
+  .mdata : AT(0xfb01000) { *(.data); } > TCM
+  .bss : { *(.bss) } > DDR
----------------
`.mdata` -> `.data`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81986