[PATCH] D99662: [AArch64] Add Machine InstCombiner patterns for FMUL indexed variant

[Andrew Savonichev via Phabricator via llvm-commits]

asavonic added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:5826
+    Opc = AArch64::FMULv8i16_indexed;
+    genIndexedMultiply(MF, MRI, TII, Root, InsInstrs, IdxDupOp, Opc, RC);
+    break;
----------------
SjoerdMeijer wrote:
> Should this be setting `MUL = genIndexedMultiply(..)`? 
This doesn't work because of the two lines below:
```
DelInstrs.push_back(MUL);
DelInstrs.push_back(&Root);
```
In case of a `DUP+FMUL` pattern, `Root` is the `FMUL` instruction we need to delete. If we also assign it to the `MUL` variable, it will be deleted twice.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:5834
   // assert(MUL && "MUL was never set");
-  DelInstrs.push_back(MUL);
+  if (MUL)
+    DelInstrs.push_back(MUL);
----------------
SjoerdMeijer wrote:
> Was wondering because of the added if here.
> Unrelated but that FIXME looks a bit dodgy. Any idea what that could be while we are at it?
This looks like a bug.
In `t32_6_3` test case, `processLogicalImmediate` function fails, so `MUL` is not updated. We also don't generate any replacement, and the `Root` gets deleted.

I think we should at least return from the function without updating `InsInstrs` and `DelInstrs` if `processLogicalImmediate` function fails.

```
(gdb) p Root.dump()
  %3:gpr32 = SUBSWri killed %2:gpr32common, 1, 0, implicit-def dead $nzcv

(gdb) p Root.Parent->dump()
bb.0 (%ir-block.0):
  liveins: $w0
  %0:gpr32 = COPY $w0
  %1:gpr32 = MOVi32imm -1431655765
  %2:gpr32common = MADDWrrr %0:gpr32, killed %1:gpr32, $wzr
  %3:gpr32 = SUBSWri killed %2:gpr32common, 1, 0, implicit-def dead $nzcv
  %4:gpr32 = exact EXTRWrri %3:gpr32, %3:gpr32, 1
  %5:gpr32 = MOVi32imm 715827883
  %6:gpr32 = SUBSWrr killed %4:gpr32, killed %5:gpr32, implicit-def $nzcv
  %7:gpr32 = CSINCWr $wzr, $wzr, 2, implicit $nzcv
  $w0 = COPY %7:gpr32
  RET_ReallyLR implicit $w0

(gdb) p Pattern
$3 = llvm::MachineCombinerPattern::MULSUBWI_OP1
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99662