[PATCH] D73117: [ARM] Mark MVE loads/store as not having side effects

[Dave Green via Phabricator via llvm-commits]

dmgreen created this revision.
dmgreen added reviewers: samparker, SjoerdMeijer, simon_tatham, ostannard.
Herald added subscribers: hiraditya, kristof.beyls.
Herald added a project: LLVM.

The hasSideEffect parameter is usually automatically inferred from instruction patters. For some of our MVE instructions, we do not have patterns though, such as for the pre/post inc loads and stores. This instead specifies the flag manually on the base MVE_VLDRSTR_base tablegen class, making sure we get this correct.

This can help with scheduling multiple loads more optimally. Here I've added a unittest as a more direct form of testing.


https://reviews.llvm.org/D73117

Files:
  llvm/lib/Target/ARM/ARMInstrMVE.td
  llvm/unittests/Target/ARM/MachineInstrTest.cpp


Index: llvm/unittests/Target/ARM/MachineInstrTest.cpp
===================================================================
--- llvm/unittests/Target/ARM/MachineInstrTest.cpp
+++ llvm/unittests/Target/ARM/MachineInstrTest.cpp
@@ -525,3 +525,53 @@
     }
   }
 }
+
+TEST(MachineInstr, HasSideEffects) {
+  using namespace ARM;
+  unsigned Opcodes[] = {
+      // Loads/Stores
+      MVE_VLDRBS16, MVE_VLDRBS16_post, MVE_VLDRBS16_pre,
+      MVE_VLDRBS32, MVE_VLDRBS32_post, MVE_VLDRBS32_pre,
+      MVE_VLDRBU16, MVE_VLDRBU16_post, MVE_VLDRBU16_pre,
+      MVE_VLDRBU32, MVE_VLDRBU32_post, MVE_VLDRBU32_pre,
+      MVE_VLDRBU8,  MVE_VLDRBU8_post,  MVE_VLDRBU8_pre,
+      MVE_VLDRHS32, MVE_VLDRHS32_post, MVE_VLDRHS32_pre,
+      MVE_VLDRHU16, MVE_VLDRHU16_post, MVE_VLDRHU16_pre,
+      MVE_VLDRHU32, MVE_VLDRHU32_post, MVE_VLDRHU32_pre,
+      MVE_VLDRWU32, MVE_VLDRWU32_post, MVE_VLDRWU32_pre,
+      MVE_VSTRB16,  MVE_VSTRB16_post,  MVE_VSTRB16_pre,
+      MVE_VSTRB32,  MVE_VSTRB32_post,  MVE_VSTRB32_pre,
+      MVE_VSTRBU8,  MVE_VSTRBU8_post,  MVE_VSTRBU8_pre,
+      MVE_VSTRH32,  MVE_VSTRH32_post,  MVE_VSTRH32_pre,
+      MVE_VSTRHU16, MVE_VSTRHU16_post, MVE_VSTRHU16_pre,
+      MVE_VSTRWU32, MVE_VSTRWU32_post, MVE_VSTRWU32_pre,
+  };
+
+  LLVMInitializeARMTargetInfo();
+  LLVMInitializeARMTarget();
+  LLVMInitializeARMTargetMC();
+
+  auto TT(Triple::normalize("thumbv8.1m.main-arm-none-eabi"));
+  std::string Error;
+  const Target *T = TargetRegistry::lookupTarget(TT, Error);
+  if (!T) {
+    dbgs() << Error;
+    return;
+  }
+
+  TargetOptions Options;
+  auto TM = std::unique_ptr<LLVMTargetMachine>(
+      static_cast<LLVMTargetMachine *>(T->createTargetMachine(
+          TT, "generic", "", Options, None, None, CodeGenOpt::Default)));
+  ARMSubtarget ST(TM->getTargetTriple(), TM->getTargetCPU(),
+                  TM->getTargetFeatureString(),
+                  *static_cast<const ARMBaseTargetMachine *>(TM.get()), false);
+  const ARMBaseInstrInfo *TII = ST.getInstrInfo();
+  auto MII = TM->getMCInstrInfo();
+
+  for (unsigned Op : Opcodes) {
+    const MCInstrDesc &Desc = TII->get(Op);
+    ASSERT_FALSE(Desc.hasUnmodeledSideEffects())
+        << MII->getName(Op) << " has unexpected side effects";
+  }
+}
Index: llvm/lib/Target/ARM/ARMInstrMVE.td
===================================================================
--- llvm/lib/Target/ARM/ARMInstrMVE.td
+++ llvm/lib/Target/ARM/ARMInstrMVE.td
@@ -5146,6 +5146,7 @@
 
   let mayLoad = dir.load;
   let mayStore = !eq(dir.load,0);
+  let hasSideEffects = 0;
   let validForTailPredication = 1;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D73117.239346.patch
Type: text/x-patch
Size: 2585 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200121/3acfa1ef/attachment.bin>