Modified compression emitter tablegen backend to emit isCompressibleInst()
check which in turn is used by getInstSizeInBytes() to better estimate
instruction size. Note the generation of compressed instructions in RISC-V
happens late in the assembler therefore instruction size estimate might be off
if computed before.
Details
Diff Detail
Event Timeline
I like the direction of this, Thanks Ana. Did you look at modifying getInstSizeInBytes? That would benefit other callers beyond the outliner.
Hi Ana, I'm yet to try this out on our benchmarks, but it looks like a good implementation to me. Considering Alex's comments above, should this patch be reduced to just the isCompressible check and I can merge the rest into the machine outliner patch after adding this one as a parent?
llvm/lib/Target/RISCV/RISCVInstrInfo.cpp | ||
---|---|---|
670 | Nitpick: this should be called isCompressibleInst (IE with an i rather than an a). |
Thanks Alex and Lewis.
I will break the patch in 3 commits:
- fix missing ImmLeaf predicate, which I will merge directly (no review).
- Compress Instr table gen back end changes. I will post this patch for review.
- Modify getInstSizeInBytes to invoke isCompressibleInst (thanks for the name fix!). This one I will run a few more correctness tests, and then post for review.
llvm/lib/Target/RISCV/RISCVInstrInfoC.td | ||
---|---|---|
141 ↗ | (On Diff #222675) | just noted a copy/paste error: I used isShiftedUInt, instead of isShiftedInt in both simm9_lsb0 and simm12_lsb0. Will fix it when committing the change. |
Added check to verify whether MI is parentless or not because isCompressible call depends of function/module info.
Rebased.
With this change, the regressions observed with the machine outliner are eliminated.
Sam, I don't think any one has reviewed yet.
Nitpick: this should be called isCompressibleInst (IE with an i rather than an a).