[PATCH] D135264: [MachineCombiner][RISCV] Enable MachineCombiner for RISCV
Anton Sidorenko via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 6 06:38:26 PDT 2022
asi-sc added inline comments.
================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:1210
+
+ if (Options.UnsafeFPMath && getFPPatterns(Root, Patterns))
+ return true;
----------------
craig.topper wrote:
> Why not use fast math flags?
I changed current implementation to use fast math flags, but I have a question related to that.
Clang distributes fast math flags to instructions if `-ffast-math` is passed, whereas llc does not. So, theoretically we can have `UnsafeFPMath` option enabled and don't have fast math flags on instructions. What do you think, should we combine instructions in this case?
================
Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:271
TargetPassConfig::addMachineSSAOptimization();
+ if (TM->getOptLevel() == CodeGenOpt::Aggressive && EnableMachineCombiner)
+ addPass(&MachineCombinerID);
----------------
craig.topper wrote:
> Doesn't every other target put this in `addILPOpts`?
Thanks for the question, it's definitely something we should discuss. You are right, `addILPOpts` is the common insertion point for this pass. However, my experiments show that it's more profitable to run machine combiner after machine LICM and machine sinking which are inserted right after ILPOpts customization point. I also need to say that my local machine combiner version has many more patterns to combine, e.g. it can reassociate integer additions.
One example I remember is that on Coremark machine sinking gathers a chain of additions in the loop:
```
bb.291.for.body3.us.us.i66:
; predecessors: %bb.289, %bb.290
successors: %bb.63(0x04000000), %bb.62(0x7c000000); %bb.63(3.12%), %bb.62(96.88%)
%76:gpr = PHI %1287:gpr, %bb.289, %1285:gpr, %bb.290
%1288:gpr = PHI %1210:gpr, %bb.289, %1286:gpr, %bb.290
%1212:gpr = ADD %1211:gpr, %72:gpr
%1223:gpr = ADD %1222:gpr, %1212:gpr
%1234:gpr = ADD %1233:gpr, %1223:gpr
%1245:gpr = ADD %1244:gpr, %1234:gpr
%1256:gpr = ADD %1255:gpr, %1245:gpr
%1267:gpr = ADD %1266:gpr, %1256:gpr
%1278:gpr = ADD %1277:gpr, %1267:gpr
%77:gpr = ADD killed %1288:gpr, killed %1278:gpr
%78:gpr = nuw ADDI %71:gpr, 8
BNE %3114:gpr, %78:gpr, %bb.62
PseudoBR %bb.63
```
Reassociation allows using multiple ALUs for this code.
At the same time I wasn't able to find any examples against inserting machine combiner a little later than other targets do.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135264/new/
https://reviews.llvm.org/D135264
More information about the llvm-commits
mailing list