This is an archive of the discontinued LLVM Phabricator instance.

Differential D141577

[4/15][Clang][RISCV][NFC] Remove unnecessary logic under RVVIntrinsic::computeBuiltinTypes
ClosedPublic

Authored by eopXD on Jan 12 2023, 12:53 AM.

Details

Reviewers
craig.topper
kito-cheng
rogfer01
frasercrmck
Commits
rGf8ec44b2a0e2: [4/15][Clang][RISCV][NFC] Remove unnecessary logic under RVVIntrinsic…
Summary

PolicyAttrs::IsUnspecified is modified in
RVV::Intrinsic::computeBuiltInTypes and used under
RVVIntrinsic::updateNamesAndPolicy. Suprisingly the modification
in RVV::Intrinsic::computeBuiltInTypes is completely unnecessary.
This commit removes the redundant logic.

This is the 4th commit of a patch-set that aims to change the default policy
for RVV intrinsics from TAMU to TAMA.

Please refer to the cover letter in the 1st commit (D141573) for an
overview.

Diff Detail

Event Timeline

eopXD created this revision.Jan 12 2023, 12:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 12 2023, 12:53 AM
Herald added subscribers: sunshaoce, VincentWu, vkmr and 27 others. · View Herald Transcript
eopXD requested review of this revision.Jan 12 2023, 12:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 12 2023, 12:53 AM
Herald added subscribers: cfe-commits, • pcwang-thead, MaskRay. · View Herald Transcript
Harbormaster completed remote builds in B207305: Diff 488515.Jan 12 2023, 12:54 AM
eopXD added a parent revision: D141575: [3/15][Clang][RISCV][NFC] Clarify edge cases of RVVIntrinsic::getSupportedMaskedPolicies for clarity.Jan 12 2023, 3:09 AM
eopXD updated this revision to Diff 488826.Jan 12 2023, 5:51 PM

Bump CI

Harbormaster completed remote builds in B207524: Diff 488826.Jan 12 2023, 6:36 PM
eopXD added a child revision: D141754: [5/15][Clang][RISCV][NFC] Remove extra attribute Policy::IntrinsicWithoutMU by reusing HasTailPolicy and HasMaskPolicy.Jan 14 2023, 4:00 AM
eopXD updated this revision to Diff 489288.Jan 14 2023, 10:59 AM

Rebase to latest main.

Harbormaster completed remote builds in B207851: Diff 489288.Jan 14 2023, 12:51 PM
eopXD retitled this revision from [WIP][4/N][Clang][RISCV][NFC] Remove unnecessary logic under RVVIntrinsic::computeBuiltinTypes to [4/15][Clang][RISCV][NFC] Remove unnecessary logic under RVVIntrinsic::computeBuiltinTypes.Jan 15 2023, 7:27 AM
eopXD edited the summary of this revision. (Show Details)
eopXD added reviewers: craig.topper, kito-cheng, rogfer01, frasercrmck.
craig.topper accepted this revision.Jan 17 2023, 4:24 PM

LGTM

clang/lib/Support/RISCVVIntrinsicUtils.cpp
921

I guess removing this negates a comment I left on the rename to Unspecified?

This revision is now accepted and ready to land.Jan 17 2023, 4:24 PM
Herald added a subscriber: luke. · View Herald TranscriptJan 17 2023, 4:24 PM
eopXD added inline comments.Jan 18 2023, 12:40 AM
clang/lib/Support/RISCVVIntrinsicUtils.cpp
921

Yes you are correct.

This revision was landed with ongoing or failed builds.Jan 24 2023, 12:09 AM
Closed by commit rGf8ec44b2a0e2: [4/15][Clang][RISCV][NFC] Remove unnecessary logic under RVVIntrinsic… (authored by eopXD). · Explain Why
This revision was automatically updated to reflect the committed changes.
eopXD added a commit: rGf8ec44b2a0e2: [4/15][Clang][RISCV][NFC] Remove unnecessary logic under RVVIntrinsic….

Revision Contents

PathSize
clang/
lib/
Support/
6 lines

Diff 491625

clang/lib/Support/RISCVVIntrinsicUtils.cpp

Show First 20 Lines • Show All 912 Lines • ▼ Show 20 Linesllvm::SmallVector<PrototypeDescriptor> RVVIntrinsic::computeBuiltinTypes(
llvm::ArrayRef<PrototypeDescriptor> Prototype, bool IsMasked, llvm::ArrayRef<PrototypeDescriptor> Prototype, bool IsMasked,
bool HasMaskedOffOperand, bool HasVL, unsigned NF, bool HasMaskedOffOperand, bool HasVL, unsigned NF,
PolicyScheme DefaultScheme, Policy PolicyAttrs) { PolicyScheme DefaultScheme, Policy PolicyAttrs) {
SmallVector<PrototypeDescriptor> NewPrototype(Prototype.begin(), SmallVector<PrototypeDescriptor> NewPrototype(Prototype.begin(),
Prototype.end()); Prototype.end());
// Update PolicyAttrs if need (TA or TAMA) for compute builtin types. // Update PolicyAttrs if need (TA or TAMA) for compute builtin types.
if (PolicyAttrs.isMAPolicy()) if (PolicyAttrs.isMAPolicy())
PolicyAttrs.TailPolicy = Policy::PolicyType::Agnostic; // TAMA PolicyAttrs.TailPolicy = Policy::PolicyType::Agnostic; // TAMA
if (PolicyAttrs.isUnspecified()) {
craig.topperUnsubmitted
Not Done
ReplyInline Actions

I guess removing this negates a comment I left on the rename to Unspecified?

craig.topper: I guess removing this negates a comment I left on the rename to Unspecified?
eopXDAuthorUnsubmitted
Done
ReplyInline Actions

Yes you are correct.

eopXD: Yes you are correct.
if (!IsMasked) {
PolicyAttrs.IsUnspecified = false;
PolicyAttrs.TailPolicy = Policy::PolicyType::Agnostic; // TA
}
}
bool HasPassthruOp = DefaultScheme == PolicyScheme::HasPassthruOperand; bool HasPassthruOp = DefaultScheme == PolicyScheme::HasPassthruOperand;
if (IsMasked) { if (IsMasked) {
// If HasMaskedOffOperand, insert result type as first input operand if // If HasMaskedOffOperand, insert result type as first input operand if
// need. // need.
if (HasMaskedOffOperand && !PolicyAttrs.isTAMAPolicy()) { if (HasMaskedOffOperand && !PolicyAttrs.isTAMAPolicy()) {
if (NF == 1) { if (NF == 1) {
NewPrototype.insert(NewPrototype.begin() + 1, NewPrototype[0]); NewPrototype.insert(NewPrototype.begin() + 1, NewPrototype[0]);
} else if (NF > 1) { } else if (NF > 1) {
▲ Show 20 Lines • Show All 183 Lines • Show Last 20 Lines