[PATCH] D104149: [MCA] Adding the CustomBehaviour class to llvm-mca

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

In D104149#2817752 <https://reviews.llvm.org/D104149#2817752>, @holland11 wrote:

> I just spoke with Quentin, and we both agree with you. If it's alright with @foad, we'd like to push the current patch with the AMDGPU example (after I finish making the modifications Andrea suggested in his first comment). Afterwards, I will submit a bug report regarding the relevant flags not being transferred from pseudo -> real instructions. This will allow me to clean up the `cnt` detection logic and make it a lot more similar to the logic found in the `si-insert-waitcnts` pass (which will make the `s_waitcnt` modelling more accurate).
>
>> I'm surprised by this. I don't see anything in the tablegen files to say that DS instructions write to M0.
>
> This is interesting. The `MCInst` for `DS_READ_U8_gfx10` definitely has a `Def` for `M0`.
>
>   		Opcode Name= DS_READ_U8_gfx10
>   		SchedClassID=4
>   		Resource Mask=0x00000000000004, Reserved=0, #Units=1, cy=1
>   		Resource Mask=0x00000000000008, Reserved=0, #Units=1, cy=1
>   		Buffer Mask=0x00000000000004
>   		Buffer Mask=0x00000000000008
>   		 Used Units=0x0000000000000c
>   		Used Groups=0x00000000000000
>   		[Def]    OpIdx=0, Latency=20, WriteResourceID=0
>   		[Def][V] OpIdx=4, Latency=20, WriteResourceID=0
>   		[Use]    OpIdx=1, UseIndex=0
>   		[Use][V] OpIdx=4, UseIndex=3
>   		MaxLatency=20
>
> The `M0` def corresponds to `[Def][V] OpIdx=4, Latency=20, WriteResourceID=0` where the `[V]` signifies that it's a 'variadic' operand (I'm not entirely sure what this means).
>
> I can submit a separate bug report for this if you'd like. It's not really related to the `CustomBehaviour` class or the `s_waitcnt` instruction.

I think this is probably a bug in mca.

According to AMDGPUGenInstrInfo.inc, DS_READ_U8_gfx10 has four operands (of which, the first one is a definition).

This is the output from llvm-mc:

  ds_read_u8 v1, v0                       ; <MCInst #6688 DS_READ_U8_gfx10
                                  ;  <MCOperand Reg:468>
                                  ;  <MCOperand Reg:467>
                                  ;  <MCOperand Imm:0>
                                  ;  <MCOperand Imm:0>
                                  ;  <MCOperand Reg:314>>

That extra register operand at the end (i.e. Reg:314) is a variadic operand.

For some reasons, mca::InstrBuilder conservatively decides to assume that Reg:314 is both defined and used.
That's a bug. It should simply check if method `MCInstrDesc::variadicOpsAreDefs()` returns true. Otherwise, it should assume that variadic operands are not definitions.
I'll see if I can fix it tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104149