[PATCH] D125515: [WebAssembly] Fix register use-def in FixIrreducibleControlFlow

[Heejin Ahn via Phabricator via llvm-commits]

aheejin created this revision.
aheejin added reviewers: dschuff, sunfish, kripken.
Herald added subscribers: pmatos, asb, wingo, ecnelises, hiraditya, jgravelle-google, sbc100, mgorny, MatzeB.
Herald added a project: All.
aheejin requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

FixIrreducibleControlFlow pass adds dispatch blocks with a `br_table`
that has multiple predecessors and successors, because it serves as
something like a traffic hub for BBs. As a result of this, there can be
register uses that are not dominated by a def in every path from the
entry block. For example, suppose register %a is defined in BB1 and used
in BB2, and there is a single path from BB1 and BB2:

  BB1 -> ... -> BB2

After FixIrreducibleControlFlow runs, there can be a dispatch block
between these two BBs:

  BB1 -> ... -> Dispatch -> ... -> BB2

And this dispatch block has multiple predecessors, now
there is a path to BB2 that does not first visit BB1, and in that path
%a is not dominated by a def anymore.

To fix this problem, we have been adding `IMPLICIT_DEF`s to all
registers in PrepareForLiveInternals pass, and then remove unnecessary
ones in OptimizeLiveIntervals pass after computing `LiveIntervals`. But
FixIrreducibleControlFlow pass itself ends up violating register use-def
relationship, resulting in invalid code. This was OK so far because
MIR verifier apparently didn't check this in validation. But @arsenm
fixed this and it caught this bug in validation
(https://github.com/llvm/llvm-project/issues/55249).

This CL moves the `IMPLICIT_DEF` adding routine from
PrepareForLiveInternals to FixIrreducibleControlFlow. We only run it
when FixIrreducibleControlFlow changes the code. And then
PrepareForLiveInternals doesn't do anything other than setting
`TracksLiveness` property, which is a prerequisite for running
`LiveIntervals` analysis, which is required by the next pass
OptimizeLiveIntervals. But I think there's no reason we can't set it
earlier; we don't seem to do anything that invalidates this. (This is
different from `LiveIntervals` analysis. Setting this only means BBs'
live-in info is correct, all uses are dominated by defs, `kill` flag is
conservatively correct, which means if there is a `kill` flag set it
should be the last use.) See
https://github.com/llvm/llvm-project/blob/2a0837aab1489c88efb03784e34c4dc9f2e28302/llvm/include/llvm/CodeGen/MachineFunction.h#L125-L134
for details.

So this CL removes PrepareForLiveInternals pass altogether. Something
similar to this was attempted by D56091 <https://reviews.llvm.org/D56091> long ago but that came short or
actually removing the pass, and I couldn't land it because
FixIrreducibleControlFlow violated use-def relationship, which this CL
fixes.

This doesn't change output in any meaningful way. All test changes
except `irreducible-cfg.mir` are register numbering.

Fixes https://github.com/llvm/llvm-project/issues/55249.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125515

Files:
  llvm/lib/Target/WebAssembly/CMakeLists.txt
  llvm/lib/Target/WebAssembly/WebAssembly.h
  llvm/lib/Target/WebAssembly/WebAssemblyFixIrreducibleControlFlow.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyOptimizeLiveIntervals.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyPrepareForLiveIntervals.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyReplacePhysRegs.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/irreducible-cfg.mir
  llvm/test/CodeGen/WebAssembly/reg-stackify.ll
  llvm/test/CodeGen/WebAssembly/umulo-128-legalisation-lowering.ll
  llvm/test/CodeGen/WebAssembly/umulo-i64.ll
  llvm/utils/gn/secondary/llvm/lib/Target/WebAssembly/BUILD.gn

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D125515.429125.patch
Type: text/x-patch
Size: 16008 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220513/d1b544d0/attachment.bin>