This is an archive of the discontinued LLVM Phabricator instance.

Differential D141764

[9/15][Clang][RISCV][NFC] Use correct type for `RVVTypeCache::computeTypes` under RISCVVEmitter.cpp
ClosedPublic

Authored by eopXD on Jan 14 2023, 7:21 AM.

Details

Reviewers
craig.topper
kito-cheng
rogfer01
frasercrmck
Commits
rG1ab7e76887e3: [9/15][Clang][RISCV][NFC] Use correct type for `RVVTypeCache::computeTypes`…
Summary

MaskedPrototype is initialized and used nowhere, this is a bug. The
existing codebase works correctly upon this bug because the default
policy for unmasked intrinsics is set to TAMU.

This is something to be fixed because when the default policy is
changed to TAMA, clang will generate incorrect result.

This is the 9th 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 14 2023, 7:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 14 2023, 7:21 AM
Herald added subscribers: sunshaoce, VincentWu, vkmr and 27 others. · View Herald Transcript
eopXD requested review of this revision.Jan 14 2023, 7:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 14 2023, 7:21 AM
Herald added subscribers: cfe-commits, • pcwang-thead, MaskRay. · View Herald Transcript
eopXD edited the summary of this revision. (Show Details)Jan 14 2023, 7:22 AM
eopXD edited the summary of this revision. (Show Details)
eopXD updated this revision to Diff 489255.Jan 14 2023, 7:48 AM

Bump CI upon update of preceeding patch.

eopXD added a child revision: D141767: [10/15][Clang][RISCV][NFC] Don't need to check for `MaskPolicy` in `isTAPolicy` and `isTUPolicy`.Jan 14 2023, 7:56 AM
eopXD updated this revision to Diff 489296.Jan 14 2023, 11:16 AM

Rebase upon latest main.

eopXD updated this revision to Diff 489304.Jan 14 2023, 11:26 AM

Dump CI.

Harbormaster completed remote builds in B207866: Diff 489304.Jan 14 2023, 12:10 PM
eopXD retitled this revision from [WIP][9/N][Clang][RISCV][NFC] Use correct type for `RVVTypeCache::computeTypes` under RISCVVEmitter.cpp to [9/15][Clang][RISCV][NFC] Use correct type for `RVVTypeCache::computeTypes` under RISCVVEmitter.cpp.Jan 15 2023, 7:50 AM
eopXD edited the summary of this revision. (Show Details)
eopXD added reviewers: craig.topper, kito-cheng, rogfer01, frasercrmck.
kito-cheng accepted this revision.Jan 16 2023, 11:31 PM

LGTM

This revision is now accepted and ready to land.Jan 16 2023, 11:31 PM
Herald added a subscriber: luke. · View Herald TranscriptJan 16 2023, 11:31 PM
craig.topper added a comment.Jan 17 2023, 4:38 PM

Description repeats a slightly different version of the same statement.

The
existing codebase works correctly upon this bug because the default
policy for unmasked intrinsics is set to TAMU.

The existing code works because the default policy for unmasked
intrinsics is set to TAMU.
eopXD edited the summary of this revision. (Show Details)Jan 24 2023, 7:00 AM
This revision was landed with ongoing or failed builds.Jan 24 2023, 7:03 AM
Closed by commit rG1ab7e76887e3: [9/15][Clang][RISCV][NFC] Use correct type for `RVVTypeCache::computeTypes`… (authored by eopXD). · Explain Why
This revision was automatically updated to reflect the committed changes.
eopXD added a commit: rG1ab7e76887e3: [9/15][Clang][RISCV][NFC] Use correct type for `RVVTypeCache::computeTypes`….

Revision Contents

PathSize
clang/
utils/
TableGen/
2 lines

Diff 491768

clang/utils/TableGen/RISCVVEmitter.cpp

Show First 20 Lines • Show All 581 Lines • ▼ Show 20 Lines for (char I : TypeRange) {
UnMaskedPolicyScheme, SupportOverloading, HasBuiltinAlias, UnMaskedPolicyScheme, SupportOverloading, HasBuiltinAlias,
ManualCodegen, *PolicyTypes, IntrinsicTypes, RequiredFeatures, ManualCodegen, *PolicyTypes, IntrinsicTypes, RequiredFeatures,
NF, P)); NF, P));
} }
if (!HasMasked) if (!HasMasked)
continue; continue;
// Create a masked intrinsic // Create a masked intrinsic
std::optional<RVVTypes> MaskTypes = std::optional<RVVTypes> MaskTypes =
TypeCache.computeTypes(BT, Log2LMUL, NF, Prototype); TypeCache.computeTypes(BT, Log2LMUL, NF, MaskedPrototype);
Out.push_back(std::make_unique<RVVIntrinsic>( Out.push_back(std::make_unique<RVVIntrinsic>(
Name, SuffixStr, OverloadedName, OverloadedSuffixStr, MaskedIRName, Name, SuffixStr, OverloadedName, OverloadedSuffixStr, MaskedIRName,
/*IsMasked=*/true, HasMaskedOffOperand, HasVL, MaskedPolicyScheme, /*IsMasked=*/true, HasMaskedOffOperand, HasVL, MaskedPolicyScheme,
SupportOverloading, HasBuiltinAlias, ManualCodegen, *MaskTypes, SupportOverloading, HasBuiltinAlias, ManualCodegen, *MaskTypes,
IntrinsicTypes, RequiredFeatures, NF, DefaultPolicy)); IntrinsicTypes, RequiredFeatures, NF, DefaultPolicy));
if (MaskedPolicyScheme == PolicyScheme::SchemeNone) if (MaskedPolicyScheme == PolicyScheme::SchemeNone)
continue; continue;
for (auto P : SupportedMaskedPolicies) { for (auto P : SupportedMaskedPolicies) {
▲ Show 20 Lines • Show All 153 Lines • Show Last 20 Lines