[PATCH] D77353: [WebAssembly] Add DW_OP_WASM_location_int

Yury Delendik via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 16 13:23:32 PDT 2020


yurydelendik added a comment.

In D77353#1987324 <https://reviews.llvm.org/D77353#1987324>, @aardappel wrote:

> @yurydelendik one thing about your patch: the variadic argument was previously declared as `SignedSizeLEB` and emitted with `emitSigned`, yet your new code reads it as `getULEB128` and stores it in an array of `uint64_t` (which we can't change since its shared code). It doesn't matter much for the current use case, but we might want to be consistent about this storing signed or unsigned values? I see value in it being signed for future use cases, but given the existing `DWARFExpression` code it may well have to be unsigned.


Previously the signed LEB was chosen as universal argument type. For wasm-local, -global, -stack we just need unsigned, so I decided to switch to be unsigned for these location. I have no preference of them be signed/unsigned.


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

https://reviews.llvm.org/D77353





More information about the llvm-commits mailing list