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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.
Herald added a subscriber: StephenFan.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h:93
+  // Does this instruction care about mask policy. If it is not, the mask policy
+  // could be either agnostic or undisturbed. For example, nomask, store and
+  // reduction operations result would not be affected by mask policy, so
----------------
nomask-> unmasked to keep with the terminology in the spec

Comma after "store"


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h:120
 enum {
   TAIL_UNDISTURBED = 0,
   TAIL_AGNOSTIC = 1,
----------------
Is 0 now TAIL_UNDISTURBED_MASK_UNDISTURBED? And 1 TAIL_AGNOSTIC_MASK_UNDISTURBED, etc. Or maybe we just remove TAIL_UNDISTURBED?


================
Comment at: llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:507
+  bool UsesMaskPolicy = RISCVII::UsesMaskPolicy(TSFlags);
+  // Could we look at the above or below instructions to choose the matched
+  // mask policy to reduce vsetvli instructions?
----------------
Is this a FIXME suggestion?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td:45
 defvar TAIL_AGNOSTIC = 1;
+defvar MASK_AGNOSTIC = 2;
 
----------------
Is this used? These names are also now ambiguous and tablegen can't easily or them


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120226