[PATCH] D140530: [RISCV] Add integer scalar instructions to isAssociativeAndCommutative

[Anton Sidorenko via Phabricator via llvm-commits]

asi-sc added a comment.

Have you had a chance to make some performance measurements?



================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:1379
+  case RISCV::XOR:
+  case RISCV::MUL:
+  case RISCV::MULW:
----------------
One thing that I'm a little worried is the described in the spec possibility for microarchitecture fusion(7.1): 
```
If both the high and low bits of the same product are required, then the recommended code sequence is: MULH[[S]U] rdh, rs1, rs2; MUL rdl, rs1, rs2 (source register specifiers must be in same order and rdh cannot be the same as rs1 or rs2). Microarchitectures can then fuse these into a single multiply operation instead of performing two separate multiplies.
```
As I know, we do nothing to support this fusion, however we quite often generate the recommended sequence (I checked some binaries I have on hand, it's not thorough research). Also some mul/mulh pairs we can easily fix by a simple DAG mutation at the scheduling stage:
```
%10:gpr = MULHU %9:gpr, %5:gpr
%11:gpr = MUL %9:gpr, %5:gpr
--->
%11:gpr = MUL %9:gpr, %5:gpr
%10:gpr = MULHU %9:gpr, %5:gpr
```

Machine combiner aggressively changes operands of multiplications, so I expect it'll worsen code from the fusion point of view.

Actually, I not sure that it's a real problem and just want to ensure that we are OK with it.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140530