[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

Mon May 31 01:17:54 PDT 2021

wingo added a comment.

In D101140#2786870 <https://reviews.llvm.org/D101140#2786870>, @jrtc27 wrote:

> In D101140#2786844 <https://reviews.llvm.org/D101140#2786844>, @wingo wrote:
>
>> In D101140#2786777 <https://reviews.llvm.org/D101140#2786777>, @jrtc27 wrote:
>>
>>> Is it just me or does having a backend-specific type in target-independent code feel wrong? It feels like there should be a space for target-specific TargetStackIDs...
>>
>> Hoo, good question.  More support for target-specific handling of stack IDs would be great.  However in this case the concept is not purely wasm-specific; I can imagine other targets that might have a similar treatment of locals (if we had a .net target, or a jvm target, or so).  The idea is that there is a separate stack consisting of named locals that may not be addressable by pointers to main memory.  In earlier drafts of this patch the name was more generic ("Object", then "Managed") but you know, our words in this area are quite overloaded.  So instead I went with something quite specific (WasmLocal) to avoid the general question -- but I do think the concept is not specific, even if it doesn't apply to any other target currently in tree.  WDYT?
>
> Well, except all the logic for it is in the backend, only the parser and the definition are in target-independent code, so even if another backend were to reuse it it would have its own completely separate logic for it, and thus there's no benefit to reusing a shared name for the thing over each target defining its own? Or would some of the code in the wasm backend be refactored out into CodeGen?

This is the case for all `TargetStackID` enumerated values except `TargetStackID::Default` -- `ScalableVector` and `SGPRSpill` have 0 uses outside target backends, though the ScalableVector name is used in RISC-V and AMDGPU.  `NoAlloc` has one mention outside the target backends but no real logic.  Still, as a reader who hacks neither on RISC-V nor AMDGPU  I find it useful that they share the same stack ID definition for scalable vectors -- lets me compare things a bit easier.

If there is a change to be made in this area (moving these enumerated values elsewhere) it would have to be in a followup I think and would need consultation with the other backends that use these values.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101140