[PATCH] D91803: [lld] Use -1 as tombstone value for discarded code ranges

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: lld/test/wasm/debug-removed-fn.ll:7
 ; CHECK: 0x0000000000000005
-; CHECK: 0x0000000000000000
+; CHECK: 0x00000000ffffffff
 
----------------
This looks suspcious - is something in the implementation (or the dumper) truncating the 64 bit -1 to a 32 bit -1? Or is this a bug in the dumper that it's printing out a 32bit value as a 64 bit value?


================
Comment at: lld/wasm/InputChunks.cpp:411-417
+      return -2;
+    } else {
+      return -1;
+    }
+  } else {
+    return 0;
+  }
----------------
Avoid "else after return" per the LLVM style guide. Also nice to reduce nesting, so I'd probably go with something like:
```
if (!.debug_)
  return 0;
if (.debug_ranges || .debug_loc)
  return -2
return -1
```


================
Comment at: lld/wasm/InputFiles.cpp:196
+    if ((isa<FunctionSymbol>(sym) || isa<DataSymbol>(sym)) && !sym->isLive()) {
+      return tombstone ? tombstone : reloc.Addend;
+    }
----------------
sbc100 wrote:
> So setting the `tombstone` to zero means we should include the `Addend` ?
Yeah, I guess that's beyond my debug info purview - but I'd suggest checking what bfd ld does here & maybe going with that, rather than what gold does/lld used to do. Which is probably absolute 0 rather than addend. For broader compatibility (& probably make this code a bit less subtle - avoiding the tombstone of zero not really being zero)


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

https://reviews.llvm.org/D91803