[PATCH] D123283: [MC] Improve st_size propagation rule

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

I think this is an improvement. No objections from me. I had a quick thought about how we might be able to support the chained assignment case although it is only based on looking at this review so it could be flawed. Ideally we'd not wait to propagate the size in `writeSymbol`. I'm guessing that it is possible to write something like

  .size x, 2
  y = x
  z = y
  .size y, 1 // should also set size of z to 1.

So it is not as simple as propagating the size on assignment. Perhaps it would be some kind of size propagation step for all symbols prior to writing them.



================
Comment at: llvm/lib/MC/ELFObjectWriter.cpp:556
+    // st_size equals y's. However, `Base` is `x` which will give us 2. Just
+    // detect the SymbolRef case which cover most needs. We'd still get wrong
+    // st_size for `z1 = z` but such chained assignments are rare.
----------------
Nit: `which covers most needs` or `which can cover most needs`


================
Comment at: llvm/lib/MC/ELFObjectWriter.cpp:558
+    // st_size for `z1 = z` but such chained assignments are rare.
+    if (Symbol.isVariable())
+      if (auto *Sym = dyn_cast<MCSymbolRefExpr>(Symbol.getVariableValue(false)))
----------------
Would it be possible to follow the chain to get to the last non-zero size. This is assuming that in the case
```
.size x, 2
y = x
.size y, 1
z = y
z1 = z
```
`*Sym = dyn_cast<MCSymbolRefExpr>(Symbol.getVariableValue(false)` for z1 will give z
and `Symbol.isVariable()` is true for `Sym`, in that case can we use another `if (auto *Sym = dyn_cast<MCSymbolRefExpr>(Symbol.getVariableValue(false))` to get to `y`, check that size is non zero and use that?

If this is possible I don't think it would be too much more complex.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123283