[PATCH] D34291: [AMDGPU] Fix illegal shrink of V_SUBB_U32 and V_ADDC_U32

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: lib/Target/AMDGPU/SIShrinkInstructions.cpp:95
       case AMDGPU::V_SUBB_U32_e64:
+        if (TII->getNamedOperand(MI, AMDGPU::OpName::src1)->isImm())
+          return false;
----------------
arsenm wrote:
> Can you add a test using a frame index and global address, those should have the same issue
I really doubt they can directly come to either of these two instructions, and I'm probably unable to forge such a source. Then it I do it in mir...

```
        %vreg4<def>, %vreg5<def> = V_SUBBREV_U32_e64 <ga:@arr>, %vreg0, %vreg3, %EXEC<imp-use>; VGPR_32:%vreg4,%vreg0 SReg_64:%vreg5,%vreg3

*** Bad machine code: Illegal immediate value for operand. ***
- function:    subbrev
- basic block: BB#0  (0x58eb3e0)
- instruction: %vreg4<def>, %vreg5<def> = V_SUBBREV_U32_e64
```

The placement in src0 is legal, that is global address is not legal for the operand, so it cannot even come to the pass I guess.

This is the OperandInfo:

```
static const MCOperandInfo OperandInfo243[] = { { AMDGPU::VGPR_32RegClassID, 0, MCOI::OPERAND_REGISTER, 0 }, { AMDGPU::SReg_64RegClassID, 0, MCOI::OPERAND_REGISTER, 0 }, { AMDGPU::VS_32RegClassID, 0, AMDGPU::OPERAND_REG_INLINE_C_INT32, 0 }, { AMDGPU::VS_32RegClassID, 0, AMDGPU::OPERAND_REG_INLINE_C_INT32, 0 }, { AMDGPU::SReg_64RegClassID, 0, AMDGPU::OPERAND_REG_INLINE_C_INT64, 0 }, };
```

verifyInstruction:

```
    case AMDGPU::OPERAND_REG_INLINE_C_INT32:
    case AMDGPU::OPERAND_REG_INLINE_C_FP32:
    case AMDGPU::OPERAND_REG_INLINE_C_INT64:
    case AMDGPU::OPERAND_REG_INLINE_C_FP64:
    case AMDGPU::OPERAND_REG_INLINE_C_INT16:
    case AMDGPU::OPERAND_REG_INLINE_C_FP16: {
      const MachineOperand &MO = MI.getOperand(i);
      if (!MO.isReg() && (!MO.isImm() || !isInlineConstant(MI, i))) {
        ErrInfo = "Illegal immediate value for operand.";
        return false;
      }
      break;
```

So neither is expected as such an input.


Repository:
  rL LLVM

https://reviews.llvm.org/D34291