[PATCH] D114573: [MachineVerifier][RFC] Make TiedOpsRewritten computable in MIRParser

Kai Luo via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 24 22:36:55 PST 2021


lkail created this revision.
lkail added reviewers: MatzeB, shchenz, efriedma, qcolombet, nemanjai, PowerPC, arsenm.
Herald added subscribers: mstorsjo, hiraditya.
lkail requested review of this revision.
Herald added subscribers: llvm-commits, wdng.
Herald added a project: LLVM.

This patch is to address post-commit comment https://reviews.llvm.org/D80538#anchor-inline-1091625. We have two ways to get it done

1. Make `TiedOpsRewritten` computable in MIRParser, like computing `isSSA`.
2. Specify a key for `TiedOpsRewritten` explicitly in the yaml.

This patch implements method 1, but I still need more inputs from reviewers which one is considered better.

With method 1, I find it hard to create test cases for it, since we don't have code check `TiedOpsRewritten` except the MachineVerifier. So I just add test cases make fast register allocator fail.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114573

Files:
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/test/CodeGen/PowerPC/tied-ops-rewritten-prop-fail.mir
  llvm/test/CodeGen/PowerPC/tied-ops-rewritten-prop.mir


Index: llvm/test/CodeGen/PowerPC/tied-ops-rewritten-prop.mir
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/tied-ops-rewritten-prop.mir
@@ -0,0 +1,23 @@
+# RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-unknown \
+# RUN:   -start-after=twoaddressinstruction -regalloc=fast %s -o - | FileCheck %s
+
+---
+name:            foo
+alignment:       16
+tracksRegLiveness: true
+frameInfo:
+  maxAlignment:    16
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $r3
+    %0:gprc = COPY $r3
+    %0 = RLWIMI killed %0, $r3, 1, 0, 30
+    $r3 = COPY %0
+    BLR8 implicit $r3, implicit $lr8, implicit $rm
+
+...
+
+# CHECK-LABEL: foo
+# CHECK: rlwimi 4, 3, 1, 0, 30
+# CHECK: blr
Index: llvm/test/CodeGen/PowerPC/tied-ops-rewritten-prop-fail.mir
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/tied-ops-rewritten-prop-fail.mir
@@ -0,0 +1,21 @@
+# RUN: not --crash llc -verify-machineinstrs -mtriple=powerpc64-unknown-unknown \
+# RUN:   -start-after=twoaddressinstruction -regalloc=fast %s -o - 2>&1 | FileCheck %s
+
+# Expect register allocator unable to handle tied-ops to fail in this test.
+---
+name:            foo
+alignment:       16
+tracksRegLiveness: true
+frameInfo:
+  maxAlignment:    16
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $r3
+    %0:gprc = COPY $r3
+    %1:gprc = RLWIMI killed %0, $r3, 1, 0, 30
+    $r3 = COPY %1
+    BLR8 implicit $r3, implicit $lr8, implicit $rm
+
+...
+# CHECK: !MO.isTied() && "tied op should be allocated"
Index: llvm/lib/CodeGen/MIRParser/MIRParser.cpp
===================================================================
--- llvm/lib/CodeGen/MIRParser/MIRParser.cpp
+++ llvm/lib/CodeGen/MIRParser/MIRParser.cpp
@@ -350,12 +350,22 @@
 
   bool HasPHI = false;
   bool HasInlineAsm = false;
+  bool AllTiedOpsRewritten = true;
   for (const MachineBasicBlock &MBB : MF) {
     for (const MachineInstr &MI : MBB) {
       if (MI.isPHI())
         HasPHI = true;
       if (MI.isInlineAsm())
         HasInlineAsm = true;
+      for (unsigned I = 0; I < MI.getNumOperands(); ++I) {
+        unsigned DefIdx;
+        const MachineOperand &MO = MI.getOperand(I);
+        if (!MO.isReg() || !MO.getReg())
+          continue;
+        if (MO.isUse() && MI.isRegTiedToDefOperand(I, &DefIdx) &&
+            MO.getReg() != MI.getOperand(DefIdx).getReg())
+          AllTiedOpsRewritten = false;
+      }
     }
   }
   if (!HasPHI)
@@ -364,8 +374,11 @@
 
   if (isSSA(MF))
     Properties.set(MachineFunctionProperties::Property::IsSSA);
-  else
+  else {
     Properties.reset(MachineFunctionProperties::Property::IsSSA);
+    if (AllTiedOpsRewritten)
+      Properties.set(MachineFunctionProperties::Property::TiedOpsRewritten);
+  }
 
   const MachineRegisterInfo &MRI = MF.getRegInfo();
   if (MRI.getNumVirtRegs() == 0)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D114573.389661.patch
Type: text/x-patch
Size: 2944 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211125/f9cc84d0/attachment.bin>


More information about the llvm-commits mailing list