[PATCH] D55347: [WebAssembly] TargetStreamer cleanup (NFC)

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added a comment.

In D55347#1325860 <https://reviews.llvm.org/D55347#1325860>, @aardappel wrote:

> We have 4 possible paths of emitting code (backend vs assembler, binary vs asm), so my answer for "where should this setType call go" is "wherever it makes those 4 paths most uniform / least fragile".
>
> That said, I'd say having the streamer setting state on symbols feels somewhat weird to me, given that a streamer is mostly a dumb byte/text writer, and not responsible for managing state (symbols are owned by MCContext, outside of the streamer). But again, the above rule is more important to me :) We have a lot of places in the code that are somewhat weird, because we're a nonstandard "CPU", so I don't think that should be of primary concern.


This patch is trying to make those 4 path more uniform. Some paths call `setType` is called during the code generation part before it reaches the streamer, and others call it during the streamer. What I suggest is, if we want to set symbols' properties (such as calling `setType` or `setModuleName`) within the streamer, we should do that for all paths. Otherwise we should do that outside of the streamer for all paths.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55347