[PATCH] D156448: [TwoAddressInstruction] Tweak constraining of tied operands w/undef source

Tue Aug 1 03:31:51 PDT 2023

foad added a comment.

> This is a rework - and arguably a partial revert - of dff3454bda097723799935e8ea7f026ff0626940 <https://reviews.llvm.org/rGdff3454bda097723799935e8ea7f026ff0626940>. That change modified the register constraint on an undef tied operand to use the regclass of the source operand. Before that change, we'd use the regclass of the operand from MCDesc.
>
> From what I can tell from that patch, the intention was to handle generic target opcodes which don't have meaningful MCDesc.

Quoting the description of D110944 <https://reviews.llvm.org/D110944>:

> before:
>
>   %16:sgpr_96 = INSERT_SUBREG undef %15:sgpr_96_with_sub0_sub1(tied-def 0), killed %11:sreg_64_xexec, %subreg.sub0_sub1
>
> After, without this patch:
>
>   undef %16.sub0_sub1:sgpr_96 = COPY killed %11:sreg_64_xexec

The problem was that sgpr_96 does not fully support sub0_sub1. //Something// should have constrained that regclass to sgpr_96_with_sub0_sub1 somehow, e.g. by calling getSubClassWithSubReg. My approach was to use the class of the tied src operand to constrain it.

> Unfortunately, the original test case appears to have been somewhat fragile. I can fully revert the original patch, and nothing changes today. So, I'm not 100% sure I actually understand it's intent.

I suspect the original repro no longer applies since D151463 <https://reviews.llvm.org/D151463> changed the way AMDGPU's sgpr_96 register classes are defined.

================
Comment at: llvm/test/CodeGen/RISCV/twoaddress-undef-regclass-constraint.mir:1
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=riscv32 -mattr=+d,+zfh,+zvfh,+v,+m -target-abi=ilp32d -run-pass twoaddressinstruction -run-pass machineverifier %s -o - | FileCheck %s
----------------
Can you precommit this or split it out to a separate patch, so we can see the diff?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156448