[PATCH] D126661: [MachineVerifier] Fix crash on early clobbered subreg operands.

[Daniil Fukalov via Phabricator via llvm-commits]

dfukalov added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:2433
+        if (SubRegIdx == 0)
+          checkLivenessAtDef(MO, MONum, DefIdx, *LI, Reg);
 
----------------
arsenm wrote:
> This does have a SubRangeCheck flag. Also wouldn't you want to perform the same checks over each subrange?
The checks over each subrange is performed in the if() just below this code.


================
Comment at: llvm/test/MachineVerifier/verifier-ec-subreg-liveness.mir:1
+# RUN: llc -mtriple amdgcn-amd-amdhsa -run-pass=liveintervals,simple-register-coalescing -verify-machineinstrs -o - %s | FileCheck %s
+# REQUIRES: amdgpu-registered-target
----------------
arsenm wrote:
> dfukalov wrote:
> > foad wrote:
> > > arsenm wrote:
> > > > dfukalov wrote:
> > > > > arsenm wrote:
> > > > > > tests in test/MachineVerifier should run none to just run the verifier. Shouldn't be running codegen passes
> > > > > The crush is observed only when the verifier runs after a pass that has preserved liveintervals analysis.
> > > > llc -march=amdgcn -run-pass liveintervals -verify-machineinstrs should work. Don't want to rely on a pass that could make changes for testing this
> > > I have a vague recollection that `-run-pass liveintervals -verify-machineinstrs` isn't enough because the pass manager frees the liveintervals analysis as soon as it is no longer required, before the verifier has a chance to run. But I could be wrong.
> > Yes, you're right, the verifier doesn't get liveinterval analysis in `-run-pass liveintervals -verify-machineinstrs` case. Regcoalescer seemed to me the smallest one from a few passes allowed to run unto crash.
> I think I figured out some trick to get live intervals preserved here but I forget what it was. I think you also need to use -verify-coalescing to get a verifier run before the pass actually runs, instead of -verify-machineinstrs
Actually in `-run-pass=liveintervals,simple-register-coalescing -verify-machineinstrs` case the verifier instance that runs into crash is performed after `liveintervals` pass and before coalescer. On the other hand `-run-pass=liveintervals,simple-register-coalescing -verify-coalescing` runs into verifier within coalescer (but before main work) so it seems a little more fragile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126661