Skip to content

Conversation

@knightXun
Copy link
Contributor

fix issue: #75301

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Dec 13, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 13, 2023

@llvm/pr-subscribers-clang

Author: flyingcat (knightXun)

Changes

fix issue: #75301


Full diff: https://github.com/llvm/llvm-project/pull/75350.diff

2 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3-1)
  • (modified) clang/lib/Sema/SemaType.cpp (+19)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 94e97a891baedc..8216861e162828 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3146,8 +3146,10 @@ def err_attribute_bad_neon_vector_size : Error< "Neon vector size must be 64 or 128 bits">; def err_attribute_invalid_sve_type : Error< "%0 attribute applied to non-SVE type %1">; +def err_attribute_x86_feature_gro_vector_size_unsupported : Error< + "vector size is not supported when '-mgeneral-regs-only' is specified">; def err_attribute_bad_sve_vector_size : Error< - "invalid SVE vector size '%0', must match value set by " + "invalid vector size '%0', must match value set by " "'-msve-vector-bits' ('%1')">; def err_attribute_arm_feature_sve_bits_unsupported : Error< "%0 is only supported when '-msve-vector-bits=<bits>' is specified with a " diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index 83610503ed9b16..0a24c9325dfa68 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -8251,6 +8251,25 @@ static void HandleVectorSizeAttr(QualType &CurType, const ParsedAttr &Attr, return; } + // check -mgeneral-regs-only is specified + const TargetInfo &targetInfo = S.getASTContext().getTargetInfo(); + llvm::Triple::ArchType arch = targetInfo.getTriple().getArch(); + const auto HasFeature = [](const clang::TargetOptions &targetOpts, + const std::string &feature) { + return std::find(targetOpts.Features.begin(), targetOpts.Features.end(), + feature) != targetOpts.Features.end(); + }; + if (CurType->isSpecificBuiltinType(BuiltinType::LongDouble)) { + if (arch == llvm::Triple::x86_64 && + HasFeature(targetInfo.getTargetOpts(), "-x87") && + HasFeature(targetInfo.getTargetOpts(), "-mmx") && + HasFeature(targetInfo.getTargetOpts(), "-sse")) { + S.Diag(Attr.getLoc(), + diag::err_attribute_x86_feature_gro_vector_size_unsupported); + Attr.setInvalid(); + return; + } + } Expr *SizeExpr = Attr.getArgAsExpr(0); QualType T = S.BuildVectorType(CurType, SizeExpr, Attr.getLoc()); if (!T.isNull()) 
@knightXun
Copy link
Contributor Author

return std::find(targetOpts.Features.begin(), targetOpts.Features.end(),
feature) != targetOpts.Features.end();
};
if (CurType->isSpecificBuiltinType(BuiltinType::LongDouble)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LongDouble is not the only problem. GCC simply disallows any vector return type without SSE (sse feature) and gives error like SSE register return with SSE disabled and give warnings to vector argument type. https://godbolt.org/z/fhG76nqYo
LLVM behaves differently from GCC. It can generate code for any vector types expect LongDouble with -mgeneral-regs-only.
I think it would be incorrect but I'm not sure if we want to make it such strict.
LongDouble cannot be supported without x87 feature. I'd expect something like https://reviews.llvm.org/D98895. It would be good if you can add vector type check there.
And you need to add test case for this scenario.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's great advice!

@RKSimon RKSimon self-requested a review December 19, 2023 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

3 participants