[PATCH] D129999: [TableGen] Mark incomplete MachineInstr as not compressible

[Piggy via Phabricator via llvm-commits]

piggynl added a comment.

Sure! I'll explain the details about why I want to make this change.

In RISCV, `j offset` is used to perform unconditional branches with a smaller-than-1MiB offset. However, for branches larger than 1MiB, where the offset is outside the bounds of a 20-bit signed integer, we must use `jump offset, rt` (which assembler will expanded to an `auipc` followed by a `jr`).

In BranchRelaxation pass, we find out-of-range offset branches and replace them with indirect branches (which is `jump` in RISCV). Since BranchRelaxation runs much later than register allocation and PEI, and compared with `j`, `jump` requires an additional scratch register `rt`, we use a RegisterScavenger to find an available register as the scratch register. https://reviews.llvm.org/D130560 handles the case where there is no scavenged register. We spill a register to the stack and use that register as the scratch register if there is no scavenged register. To reach that, we have to reserve an emergency spill slot in the stack.

PEI pass first calls `TargetFrameLowering::processFunctionBeforeFrameFinalized()`. Then `PEI::replaceFrameIndices()` replaces all FrameIndex MachineOperand with physical register references and actual offsets. We add the spill slot in `processFunctionBeforeFrameFinalized()` if we estimate the function has a large size (which means it is possible that have an out-of-range branch). We iterate all MachineInstr in the MachineFunction, calls `TargetInstrInfo::getInstSizeInBytes()`, and use the sum of the results as the estimated function size. Due to the fact that this step is run prior to `replaceFrameIndices()`, FrameIndex MachineOperand may be present in MachineInstr when determining the size of MachineInstr. Without this patch, if a MachineInstr expects a MachineOperand to be Reg but is actually FrameIndex, `isCompressibleInst()` will terminate at `MachineOperandType::getReg()`.

I added the `.isReg()` check in this patch to make `isCompressibleInst()` return false for these MachineInstr, allowing `getInstSizeInBytes()` to return a value and `EstimateFunctionSizeInBytes()` to work as intended.


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

https://reviews.llvm.org/D129999