[PATCH] D142777: [RISCV] Add asserts that we don't increase codesize during macro expansion

[Philip Reames via Phabricator via llvm-commits]

reames created this revision.
reames added reviewers: craig.topper, asb, luke, kito-cheng.
Herald added subscribers: VincentWu, vkmr, frasercrmck, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, niosHD, sabuasal, bollu, simoncook, johnrusso, rbar, hiraditya, arichardson, mcrosier.
Herald added a project: All.
reames requested review of this revision.
Herald added subscribers: pcwang-thead, eopXD, MaskRay.
Herald added a project: LLVM.

We currently assume that macro expansion doesn't increase the distance between a branch and it's label.  This patch adds some asserts to catch violations of this property in macro expansion.

I chose to only do the assertion at the function level as we have to scan the whole function for size changes (since expansion can create multiple blocks).  We could cache individual block sizes, and thus make the check cheap enough to do after every expansion, but that requires more complex code.

It also looks like we have some shared code structure here.  If reviewers want, I'm happy to factor out a base class for expansion, and add the asserts there.  I'm not sure this is worth doing, but will happily defer to reviewer preference.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142777

Files:
  llvm/lib/Target/RISCV/RISCVExpandAtomicPseudoInsts.cpp
  llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp


Index: llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
===================================================================
--- llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
+++ llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
@@ -56,9 +56,21 @@
 
 bool RISCVExpandPseudo::runOnMachineFunction(MachineFunction &MF) {
   TII = static_cast<const RISCVInstrInfo *>(MF.getSubtarget().getInstrInfo());
+
+  unsigned OldSize = 0;
+  for (auto &MBB : MF)
+    for (auto &MI : MBB)
+      OldSize += TII->getInstSizeInBytes(MI);
+
   bool Modified = false;
   for (auto &MBB : MF)
     Modified |= expandMBB(MBB);
+
+  unsigned NewSize = 0;
+  for (auto &MBB : MF)
+    for (auto &MI : MBB)
+      NewSize += TII->getInstSizeInBytes(MI);
+  assert(OldSize >= NewSize);
   return Modified;
 }
 
@@ -277,9 +289,21 @@
 
 bool RISCVPreRAExpandPseudo::runOnMachineFunction(MachineFunction &MF) {
   TII = static_cast<const RISCVInstrInfo *>(MF.getSubtarget().getInstrInfo());
+
+  unsigned OldSize = 0;
+  for (auto &MBB : MF)
+    for (auto &MI : MBB)
+      OldSize += TII->getInstSizeInBytes(MI);
+
   bool Modified = false;
   for (auto &MBB : MF)
     Modified |= expandMBB(MBB);
+
+  unsigned NewSize = 0;
+  for (auto &MBB : MF)
+    for (auto &MI : MBB)
+      NewSize += TII->getInstSizeInBytes(MI);
+  assert(OldSize >= NewSize);
   return Modified;
 }
 
Index: llvm/lib/Target/RISCV/RISCVExpandAtomicPseudoInsts.cpp
===================================================================
--- llvm/lib/Target/RISCV/RISCVExpandAtomicPseudoInsts.cpp
+++ llvm/lib/Target/RISCV/RISCVExpandAtomicPseudoInsts.cpp
@@ -64,9 +64,21 @@
 
 bool RISCVExpandAtomicPseudo::runOnMachineFunction(MachineFunction &MF) {
   TII = static_cast<const RISCVInstrInfo *>(MF.getSubtarget().getInstrInfo());
+
+  unsigned OldSize = 0;
+  for (auto &MBB : MF)
+    for (auto &MI : MBB)
+      OldSize += TII->getInstSizeInBytes(MI);
+
   bool Modified = false;
   for (auto &MBB : MF)
     Modified |= expandMBB(MBB);
+
+  unsigned NewSize = 0;
+  for (auto &MBB : MF)
+    for (auto &MI : MBB)
+      NewSize += TII->getInstSizeInBytes(MI);
+  assert(OldSize >= NewSize);
   return Modified;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D142777.492853.patch
Type: text/x-patch
Size: 2168 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230127/f57e44ad/attachment.bin>