[PATCH] D81758: [WebAssembly] Handle unstackified TEE dest in ExplicitLocals

[Derek Schuff via Phabricator via llvm-commits]

dschuff added a comment.

Maybe we could keep using "unstackify" as a verb and use "non-stackified" as an adjective. I like that better than "destackify" anyway.

> Not sure what you mean by the invariant, but that means if one of `tee`'s dest is stackified and the other is not, that violation can't be avoided because of the reason above. Where we check and deal with that invariant is ExplicitLocals, which this CL fixes.

Yeah, I would think that anytime there is a `tee`, one of its dest would be stackified, because that's the point of having a tee in the first place.

I get that we have to do the BB-splitting you mentioned. Would it be possible to replace the tee with a copy?
e.g.

  bb0:
    r0 = copy defreg
    ... use defreg (instead of r1)
    try:
    call @bar
  
  ...
  use use non-stackified r0

or if that's not possible, maybe even inserting a copy directly after the tee that immediately consumes the stackified r0 and sets a non-stackified reg would be ok, since the final result (after copies are eliminated) should probably be the same.



================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp:277
+        // operand 0 is stackified and operand 1 is not. But it is possible that
+        // operand 0 is unstackified in fixUnwindMismatches function in
+        // CFGStackify pass when a nested try-catch-end is introduced. In this
----------------
aheejin wrote:
> dschuff wrote:
> > does this mean that fixUnwindMismatches makes it unstackified or that it's already unstackified when fixUnwindMismatches runs?
> > edit:
> > I think it means the former, so maybe this could say "operand 0 becomes unstackified" instead. I guess this is all because "unstackified" could be a noun or a verb.... thanks English...
> Yeah it means former. Changed to 'becomes' as you suggested. And.. isn't it an adjective or a verb..?
Yeah, I'm just saying it could be used as either, which is what can make things confusing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81758