[PATCH] D120226: [RISCV] Support mask policy for RVV IR intrinsics.

[ShihPo Hung via Phabricator via llvm-commits]

arcbbb added inline comments.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h:120
 enum {
   TAIL_UNDISTURBED = 0,
   TAIL_AGNOSTIC = 1,
----------------
khchen wrote:
> arcbbb wrote:
> > craig.topper wrote:
> > > Is 0 now TAIL_UNDISTURBED_MASK_UNDISTURBED? And 1 TAIL_AGNOSTIC_MASK_UNDISTURBED, etc. Or maybe we just remove TAIL_UNDISTURBED?
> > Sorry I didn't notice this change.
> > I saw there are two places using 0 as TAIL_UNDISTURBED.
> > https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td 
> > It looks like policy is a 2-bit value representing TU_MU, TA_MU, TU_MA, and TA_MA as below
> > | TU_MU | 0
> > |TA_MU | 1
> > |TU_MA | 2
> > |TA_MA | 3
> > Maybe we still need the TU definition in enum.
> Do you mean in order to make code more consistent (enum in `RISCVBaseInfo.h` and `TAIL_UNDISTURBED` defined in td) we need to add `TAIL_UNDISTURBED` back to enum?
> I think it's ok to me, but we should also update them as explicitly name like `TU_MU` or `TAIL_UNDISTURBED_MASK_UNDISTURBED` since we start model mask policy now?
> 
I got it. I thought `0` was undefined at my first glance on the enum.
I think it's fine to leave a comment here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120226