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

[Heejin Ahn via Phabricator via llvm-commits]

aheejin marked 2 inline comments as done.
aheejin added a comment.

In D81758#2090825 <https://reviews.llvm.org/D81758#2090825>, @dschuff wrote:

> (I'll use the term "de-stackify" as a verb to avoid confusion).


The relevant function name in CFGStackify is `unstackifyVRegsUsedInSplitBB`. Do you think this is better to be renamed to `destackify-` as well?

> If fixUnwindMismatches de-stackifies the tee result, does that violate some kind of invariant? is it better to fix it up in fixUnwindMismatches?

That unstackifying behavior was added in D68218 <https://reviews.llvm.org/D68218>. This happens because, for example,

  bb0:
    r0, r1 = tee defreg
    ...
    call @bar
    ...
    use stackified r0

This basic block can be split while we introduce inner try-catch for `call @bar`, in case its unwind destination mismatches. Then this becomes something like

  bb0:
    r0, r1 = tee defreg
    ...
    try
    call @bar
  
  bb1:
    catch
    ...
  
  bb2:
    end_try
    ...
    use stackified r0??

We do register stackification only within a BB, so r0 cannot be stackified at this point. This is the reason why we unstackify it there. I don't think we can fix it in `fixUnwindMismatches` as long as we split BBs there.

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.



================
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
----------------
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..?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81758