[PATCH] D30038: [ADMGPU] SDWA peephole optimization pass.

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: lib/Target/AMDGPU/SIPeepholeSDWA.cpp:276
+        continue;
+      } else if (PotentialMI != PotentialMO.getParent()) {
+        return nullptr;
----------------
You only need to do:

```
if (PotentialMI != PotentialMO.getParent()) {
  return nullptr;
```
If != nullptr and continue are not needed.


================
Comment at: lib/Target/AMDGPU/SIPeepholeSDWA.cpp:311
+      // src2. This is not allowed.
+      return;
+    }
----------------
Looks like parent will be erased anyway. I assume convertToSDWA should return status.


================
Comment at: lib/Target/AMDGPU/SIPeepholeSDWA.cpp:579
+  MachineInstrBuilder SDWAInst =
+    BuildMI(*MI.getParent(), MI, MI.getDebugLoc(),SDWADesc);
+
----------------
Space after comma.


================
Comment at: lib/Target/AMDGPU/SIPeepholeSDWA.cpp:579
+  MachineInstrBuilder SDWAInst =
+    BuildMI(*MI.getParent(), MI, MI.getDebugLoc(),SDWADesc);
+
----------------
rampitec wrote:
> Space after comma.
You are working in pre-regalloc... I think you need to update LIS when you create or delete instructions.


================
Comment at: lib/Target/AMDGPU/SIPeepholeSDWA.cpp:646
+  for (auto &Operand : SDWAOperands) {
+    Operand->convertToSDWA(*SDWAInst, TII);
+  }
----------------
There can be only one SDWA operand, right? It seems this loop may try to create more than one...


https://reviews.llvm.org/D30038