eopXD added inline comments.
================
Comment at: clang/lib/Support/RISCVVIntrinsicUtils.cpp:1032
+ if (IsMasked) {
+ if (PolicyAttrs.isTUMAPolicy() && !HasMaskPolicy)
+ appendPolicySuffix("_tum");
----------------
craig.topper wrote:
> If you hadn't just removed Omit, it somewhat feels like Omit should be part
> of Policy and this code to pick the policy suffix string should be inside the
> Policy class. But I'll defer to your judgement.
Good idea, I will encapsulate logics into `Policy`. Considering the next
patch-set, to simplify the policy suffixes, `IsUnspecified` can be further
removed. Eventually we can have something like:
```
void RVVIntrinsic::updateNamesAndPolicy(bool IsMasked, bool HasPolicy,
std::string &Name,
std::string &BuiltinName,
std::string &OverloadedName,
Policy &PolicyAttrs) {
std::string PolicySuffix =
PolicyAttrs.getPolicySuffix(IsMasked, HasPolicy);
Name += PolicySuffix;
BuiltinName += PolicySuffix;
OverloadedName += PolicySuffix;
}
```
I will create a separate revision so we won't be distracted towards the goal of
this patch-set.
---
On the other hand, here is my thought process when considering the removal of
`Omit` as a legit refactoring.
Logics to deal with `Omit` occurs under here (`updateNamesAndPolicy`),
`computeBuiltinTypes`, and `getPolicyAttrs`. The `Omit` state specified to a
`Policy` implies it has to be corrected to something specific (either Agnostic
or Undisturbed) before calling `getPolicyAttrs`, which will set the values in
`riscv_vector_cg` (called under `RISCVVEmitter.cpp`. This blocks the
possibility of setting `TailPolicy`, `MaskPolicy` as constant members.
I picked `HasTailPolicy` and `HasMaskPolicy` for the predicates because it was
exactly how `riscv_vector.td` defined the intrinsics. They are necessary as
conditions now with the current policy suffix naming rules. With the next
patch-set, `HasTailPolicy` and `HasMaskPolicy` will only be called by
`getSupportedUnMaskedPolicies` and `getSupportedMaskedPolicies`. This is
because the policy suffix logics here can be simplified to something like:
(this does not contradict with the encapsulating topic above)
```
if (PolicyAttrs.isUnspecified()) {
if (IsMasked) {
Name += "_m";
if (HasPolicy)
BuiltinName += "_tama";
else
BuiltinName += "_m";
} else {
if (HasPolicy)
BuiltinName += "_ta";
}
} else {
if (IsMasked) {
if (PolicyAttrs.isTAMUPolicy())
appendPolicySuffix("_mu")
else if (PolicyAttrs.isTUMAPolicy())
appendPolicySuffix("_tum")
else if (PolicyAttrs.isTUMUPolicy())
appendPolicySuffix("_tumu")
else
llvm_unreachable("Unhandled policy condition");
} else {
if (PolicyAttrs.isTUPolicy())
appendPolicySuffix("_tu");
else
llvm_unreachable("Unhandled policy condition");
}
}
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141768/new/
https://reviews.llvm.org/D141768
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
