[llvm] [GlobalISel][TableGen] Take first result for multi-output instructions (PR #81130)

Björn Pettersson via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 15 09:33:45 PST 2024


bjope wrote:

> LGTM but wait for @arsenm to comment, I'm really wondering about those instselect test changes

I think that it somewhat is related to this comment:
```
bool AMDGPUInstructionSelector::selectG_FNEG(MachineInstr &MI) const {
  // Only manually handle the f64 SGPR case.
  //
  // FIXME: This is a workaround for 2.5 different tablegen problems. Because
  // the bit ops theoretically have a second result due to the implicit def of
  // SCC, the GlobalISelEmitter is overly conservative and rejects it. Fixing
  // that is easy by disabling the check. The result works, but uses a
  // nonsensical sreg32orlds_and_sreg_1 regclass.
  //
  // The DAG emitter is more problematic, and incorrectly adds both S_XOR_B32 to
  // the variadic REG_SEQUENCE operands.
```
And for example AMDGPUInstructionSelector::selectG_FABS refer to also being a fix for the same problem.

I guess that this patch fixes the problem with the implicit def. But I don't know enough about AMDGPU to understand the consequence here. And if fxing the tablegen problem makes the result worse here or not. Neither if the FIXME should be updated or removed (nor if the workarounds in AMDGPUInstructionSelector can be removed after this patch).

@Pierre-vh : I agree that it would be nice with some kind of feedback from @arsenm as well.

https://github.com/llvm/llvm-project/pull/81130


More information about the llvm-commits mailing list