[PATCH] D95510: Diagnose if a SLEB128 is too large to fit in an int64_t.

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
Comment at: llvm/lib/ObjectYAML/WasmEmitter.cpp:581
+      encodeSLEB128(Reloc.Addend, OS);
+      break;
     }
----------------
sbc100 wrote:
> sbc100 wrote:
> > I'm not sure about this part.   The addend is currently defined to be the same data type regardless of the encoding of the relocation itself. 
> > 
> > The addend is the value is added to the to the value before writing to relocation location.  The encoding used when writing to the relocation address (LEB vs SLEB vs I32, etc) I think is independent of this.  
> > 
> > See the definition of addend in https://github.com/WebAssembly/tool-conventions/blob/master/Linking.md
> Regardless of the discussion here it might make sense to split out this wasm-specific part of the change.
It looks like perhaps the correct fix it to use encodeSLEB128 universally here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95510