[PATCH] D139995: [RISCV] Refactor RVV Policy by structure

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.


================
Comment at: clang/include/clang/Support/RISCVVIntrinsicUtils.h:105
+      : TU(_TU), TA(_TA), MU(_MU), MA(_MA) {}
+  Policy(bool _TU, bool _TA, bool _MU, bool _MA, bool _IntrinsicWithoutMU)
+      : TU(_TU), TA(_TA), MU(_MU), MA(_MA),
----------------
You can unify the two constructors by adding `= false` after `_IntrinsicWithoutMU`


================
Comment at: clang/include/clang/Support/RISCVVIntrinsicUtils.h:133
+    return this->PolicyNone == Other.PolicyNone && this->TU == Other.TU &&
+           this->TA == Other.TA && this->MU == Other.MU && this->MA == Other.MA;
+  }
----------------
This is missing `IntrinsicWithoutMU`


================
Comment at: clang/include/clang/Support/RISCVVIntrinsicUtils.h:140
+    // Just for maintain the old order for quick test.
+    return std::tie(this->MU, this->MA, this->TA, this->TU) <
+           std::tie(Other.MU, Other.MA, Other.TA, Other.TU);
----------------
Does this need to include PolicyNone and IntrinsicWithoutMU?


================
Comment at: clang/lib/Sema/SemaRISCVVectorLookup.cpp:211
+    llvm::SmallVector<Policy> SupportedUnMaskedPolicies;
+    SupportedUnMaskedPolicies.push_back(
+        Policy(true, false, false, false)); // TU
----------------
Use emplace_back


================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:530
+    llvm::SmallVector<Policy> SupportedUnMaskedPolicies;
+    SupportedUnMaskedPolicies.push_back(
+        Policy(true, false, false, false)); // TU
----------------
emplace_back


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139995