[clang] handle fp options in __builtin_convertvector #125522
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: Jakub Ficek (ficol) ChangesFull diff: https://github.com/llvm/llvm-project/pull/125522.diff 12 Files Affected:
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 7be4022649329b..1e944277483548 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -4579,25 +4579,97 @@ class ShuffleVectorExpr : public Expr {
/// ConvertVectorExpr - Clang builtin function __builtin_convertvector
/// This AST node provides support for converting a vector type to another
/// vector type of the same arity.
-class ConvertVectorExpr : public Expr {
+class ConvertVectorExpr final
+ : public Expr,
+ private llvm::TrailingObjects<ConvertVectorExpr, FPOptionsOverride> {
private:
Stmt *SrcExpr;
TypeSourceInfo *TInfo;
SourceLocation BuiltinLoc, RParenLoc;
+ friend TrailingObjects;
friend class ASTReader;
friend class ASTStmtReader;
- explicit ConvertVectorExpr(EmptyShell Empty) : Expr(ConvertVectorExprClass, Empty) {}
+ explicit ConvertVectorExpr(bool HasFPFeatures, EmptyShell Empty)
+ : Expr(ConvertVectorExprClass, Empty) {
+ ConvertVectorExprBits.HasFPFeatures = HasFPFeatures;
+ }
-public:
ConvertVectorExpr(Expr *SrcExpr, TypeSourceInfo *TI, QualType DstType,
ExprValueKind VK, ExprObjectKind OK,
- SourceLocation BuiltinLoc, SourceLocation RParenLoc)
+ SourceLocation BuiltinLoc, SourceLocation RParenLoc,
+ FPOptionsOverride FPFeatures)
: Expr(ConvertVectorExprClass, DstType, VK, OK), SrcExpr(SrcExpr),
TInfo(TI), BuiltinLoc(BuiltinLoc), RParenLoc(RParenLoc) {
+ ConvertVectorExprBits.HasFPFeatures = FPFeatures.requiresTrailingStorage();
+ if (hasStoredFPFeatures())
+ setStoredFPFeatures(FPFeatures);
setDependence(computeDependence(this));
}
+ size_t numTrailingObjects(OverloadToken<FPOptionsOverride>) const {
+ return ConvertVectorExprBits.HasFPFeatures ? 1 : 0;
+ }
+
+ FPOptionsOverride &getTrailingFPFeatures() {
+ assert(ConvertVectorExprBits.HasFPFeatures);
+ return *getTrailingObjects<FPOptionsOverride>();
+ }
+
+ const FPOptionsOverride &getTrailingFPFeatures() const {
+ assert(ConvertVectorExprBits.HasFPFeatures);
+ return *getTrailingObjects<FPOptionsOverride>();
+ }
+
+public:
+ static ConvertVectorExpr *CreateEmpty(const ASTContext &C,
+ bool hasFPFeatures);
+
+ static ConvertVectorExpr *Create(const ASTContext &C, Expr *SrcExpr,
+ TypeSourceInfo *TI, QualType DstType,
+ ExprValueKind VK, ExprObjectKind OK,
+ SourceLocation BuiltinLoc,
+ SourceLocation RParenLoc,
+ FPOptionsOverride FPFeatures);
+
+ /// Get the FP contractibility status of this operator. Only meaningful for
+ /// operations on floating point types.
+ bool isFPContractableWithinStatement(const LangOptions &LO) const {
+ return getFPFeaturesInEffect(LO).allowFPContractWithinStatement();
+ }
+
+ /// Is FPFeatures in Trailing Storage?
+ bool hasStoredFPFeatures() const {
+ return ConvertVectorExprBits.HasFPFeatures;
+ }
+
+ /// Get FPFeatures from trailing storage.
+ FPOptionsOverride getStoredFPFeatures() const {
+ return getTrailingFPFeatures();
+ }
+
+ /// Get the store FPOptionsOverride or default if not stored.
+ FPOptionsOverride getStoredFPFeaturesOrDefault() const {
+ return hasStoredFPFeatures() ? getStoredFPFeatures() : FPOptionsOverride();
+ }
+
+ /// Set FPFeatures in trailing storage, used by Serialization & ASTImporter.
+ void setStoredFPFeatures(FPOptionsOverride F) { getTrailingFPFeatures() = F; }
+
+ /// Get the FP features status of this operator. Only meaningful for
+ /// operations on floating point types.
+ FPOptions getFPFeaturesInEffect(const LangOptions &LO) const {
+ if (ConvertVectorExprBits.HasFPFeatures)
+ return getStoredFPFeatures().applyOverrides(LO);
+ return FPOptions::defaultWithoutTrailingStorage(LO);
+ }
+
+ FPOptionsOverride getFPOptionsOverride() const {
+ if (ConvertVectorExprBits.HasFPFeatures)
+ return getStoredFPFeatures();
+ return FPOptionsOverride();
+ }
+
/// getSrcExpr - Return the Expr to be converted.
Expr *getSrcExpr() const { return cast<Expr>(SrcExpr); }
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index 405c6166adb15a..604ac51d478cf4 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -1215,6 +1215,20 @@ class alignas(void *) Stmt {
SourceLocation Loc;
};
+ class ConvertVectorExprBitfields {
+ friend class ConvertVectorExpr;
+
+ LLVM_PREFERRED_TYPE(ExprBitfields)
+ unsigned : NumExprBits;
+
+ //
+ /// This is only meaningful for operations on floating point
+ /// types when additional values need to be in trailing storage.
+ /// It is 0 otherwise.
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned HasFPFeatures : 1;
+ };
+
union {
// Same order as in StmtNodes.td.
// Statements
@@ -1293,6 +1307,7 @@ class alignas(void *) Stmt {
// Clang Extensions
OpaqueValueExprBitfields OpaqueValueExprBits;
+ ConvertVectorExprBitfields ConvertVectorExprBits;
};
public:
diff --git a/clang/include/clang/AST/TextNodeDumper.h b/clang/include/clang/AST/TextNodeDumper.h
index bfd205ffb0d99a..632263bd9badf5 100644
--- a/clang/include/clang/AST/TextNodeDumper.h
+++ b/clang/include/clang/AST/TextNodeDumper.h
@@ -424,6 +424,7 @@ class TextNodeDumper
void VisitOpenACCAsteriskSizeExpr(const OpenACCAsteriskSizeExpr *S);
void VisitEmbedExpr(const EmbedExpr *S);
void VisitAtomicExpr(const AtomicExpr *AE);
+ void VisitConvertVectorExpr(const ConvertVectorExpr *S);
};
} // namespace clang
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index c9f2f905d2134c..feaf2941aae625 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -7386,9 +7386,10 @@ ExpectedStmt ASTNodeImporter::VisitConvertVectorExpr(ConvertVectorExpr *E) {
if (Err)
return std::move(Err);
- return new (Importer.getToContext())
- ConvertVectorExpr(ToSrcExpr, ToTSI, ToType, E->getValueKind(),
- E->getObjectKind(), ToBuiltinLoc, ToRParenLoc);
+ return ConvertVectorExpr::Create(
+ Importer.getToContext(), ToSrcExpr, ToTSI, ToType, E->getValueKind(),
+ E->getObjectKind(), ToBuiltinLoc, ToRParenLoc,
+ E->getStoredFPFeaturesOrDefault());
}
ExpectedStmt ASTNodeImporter::VisitShuffleVectorExpr(ShuffleVectorExpr *E) {
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 4fc62919fde94b..1a6e6f6bdafb6d 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -3899,6 +3899,8 @@ FPOptions Expr::getFPFeaturesInEffect(const LangOptions &LO) const {
return BO->getFPFeaturesInEffect(LO);
if (auto Cast = dyn_cast<CastExpr>(this))
return Cast->getFPFeaturesInEffect(LO);
+ if (auto ConvertVector = dyn_cast<ConvertVectorExpr>(this))
+ return ConvertVector->getFPFeaturesInEffect(LO);
return FPOptions::defaultWithoutTrailingStorage(LO);
}
@@ -5439,3 +5441,21 @@ OpenACCAsteriskSizeExpr *
OpenACCAsteriskSizeExpr::CreateEmpty(const ASTContext &C) {
return new (C) OpenACCAsteriskSizeExpr({}, C.IntTy);
}
+
+ConvertVectorExpr *ConvertVectorExpr::CreateEmpty(const ASTContext &C,
+ bool hasFPFeatures) {
+ void *Mem = C.Allocate(totalSizeToAlloc<FPOptionsOverride>(hasFPFeatures),
+ alignof(ConvertVectorExpr));
+ return new (Mem) ConvertVectorExpr(hasFPFeatures, EmptyShell());
+}
+
+ConvertVectorExpr *ConvertVectorExpr::Create(
+ const ASTContext &C, Expr *SrcExpr, TypeSourceInfo *TI, QualType DstType,
+ ExprValueKind VK, ExprObjectKind OK, SourceLocation BuiltinLoc,
+ SourceLocation RParenLoc, FPOptionsOverride FPFeatures) {
+ bool HasFPFeatures = FPFeatures.requiresTrailingStorage();
+ unsigned Size = totalSizeToAlloc<FPOptionsOverride>(HasFPFeatures);
+ void *Mem = C.Allocate(Size, alignof(ConvertVectorExpr));
+ return new (Mem) ConvertVectorExpr(SrcExpr, TI, DstType, VK, OK, BuiltinLoc,
+ RParenLoc, FPFeatures);
+}
diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp
index 10d7e4c0c73872..26520914e8e85f 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -3055,3 +3055,9 @@ void TextNodeDumper::VisitEmbedExpr(const EmbedExpr *S) {
void TextNodeDumper::VisitAtomicExpr(const AtomicExpr *AE) {
OS << ' ' << AE->getOpAsString();
}
+
+void TextNodeDumper::VisitConvertVectorExpr(const ConvertVectorExpr *S) {
+ VisitStmt(S);
+ if (S->hasStoredFPFeatures())
+ printFPOptions(S->getStoredFPFeatures());
+}
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index df850421c72c6c..635bb31ab28b7e 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -1949,6 +1949,7 @@ Value *ScalarExprEmitter::VisitConvertVectorExpr(ConvertVectorExpr *E) {
llvm::Value *Zero = llvm::Constant::getNullValue(SrcTy);
if (SrcEltTy->isFloatingPointTy()) {
+ CodeGenFunction::CGFPOptionsRAII FPOptions(CGF, E);
return Builder.CreateFCmpUNE(Src, Zero, "tobool");
} else {
return Builder.CreateICmpNE(Src, Zero, "tobool");
@@ -1975,6 +1976,7 @@ Value *ScalarExprEmitter::VisitConvertVectorExpr(ConvertVectorExpr *E) {
} else {
assert(SrcEltTy->isFloatingPointTy() && DstEltTy->isFloatingPointTy() &&
"Unknown real conversion");
+ CodeGenFunction::CGFPOptionsRAII FPOptions(CGF, E);
if (DstEltTy->getTypeID() < SrcEltTy->getTypeID())
Res = Builder.CreateFPTrunc(Src, DstTy, "conv");
else
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 61b2c8cf1cad72..a31f019e849350 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5261,8 +5261,8 @@ ExprResult Sema::ConvertVectorExpr(Expr *E, TypeSourceInfo *TInfo,
<< E->getSourceRange());
}
- return new (Context) class ConvertVectorExpr(E, TInfo, DstTy, VK, OK,
- BuiltinLoc, RParenLoc);
+ return ConvertVectorExpr::Create(Context, E, TInfo, DstTy, VK, OK, BuiltinLoc,
+ RParenLoc, CurFPFeatureOverrides());
}
bool Sema::BuiltinPrefetch(CallExpr *TheCall) {
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
index dc953ddeee85c7..a6c659995d3b61 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -1387,10 +1387,15 @@ void ASTStmtReader::VisitShuffleVectorExpr(ShuffleVectorExpr *E) {
void ASTStmtReader::VisitConvertVectorExpr(ConvertVectorExpr *E) {
VisitExpr(E);
+ bool HasFPFeatures = CurrentUnpackingBits->getNextBit();
+ assert(HasFPFeatures == E->hasStoredFPFeatures());
E->BuiltinLoc = readSourceLocation();
E->RParenLoc = readSourceLocation();
E->TInfo = readTypeSourceInfo();
E->SrcExpr = Record.readSubExpr();
+ if (HasFPFeatures)
+ E->setStoredFPFeatures(
+ FPOptionsOverride::getFromOpaqueInt(Record.readInt()));
}
void ASTStmtReader::VisitBlockExpr(BlockExpr *E) {
@@ -3391,9 +3396,13 @@ Stmt *ASTReader::ReadStmtFromStream(ModuleFile &F) {
S = new (Context) ShuffleVectorExpr(Empty);
break;
- case EXPR_CONVERT_VECTOR:
- S = new (Context) ConvertVectorExpr(Empty);
+ case EXPR_CONVERT_VECTOR: {
+ BitsUnpacker ConvertVectorExprBits(Record[ASTStmtReader::NumStmtFields]);
+ ConvertVectorExprBits.advance(ASTStmtReader::NumExprBits);
+ bool HasFPFeatures = ConvertVectorExprBits.getNextBit();
+ S = ConvertVectorExpr::CreateEmpty(Context, HasFPFeatures);
break;
+ }
case EXPR_BLOCK:
S = new (Context) BlockExpr(Empty);
diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp
index e5caf3debc0232..a26f628dd703a6 100644
--- a/clang/lib/Serialization/ASTWriterStmt.cpp
+++ b/clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1335,11 +1335,15 @@ void ASTStmtWriter::VisitShuffleVectorExpr(ShuffleVectorExpr *E) {
void ASTStmtWriter::VisitConvertVectorExpr(ConvertVectorExpr *E) {
VisitExpr(E);
+ bool HasFPFeatures = E->hasStoredFPFeatures();
+ CurrentPackingBits.addBit(HasFPFeatures);
Record.AddSourceLocation(E->getBuiltinLoc());
Record.AddSourceLocation(E->getRParenLoc());
Record.AddTypeSourceInfo(E->getTypeSourceInfo());
Record.AddStmt(E->getSrcExpr());
Code = serialization::EXPR_CONVERT_VECTOR;
+ if (HasFPFeatures)
+ Record.push_back(E->getStoredFPFeatures().getAsOpaqueInt());
}
void ASTStmtWriter::VisitBlockExpr(BlockExpr *E) {
diff --git a/clang/test/AST/ast-dump-fpfeatures.cpp b/clang/test/AST/ast-dump-fpfeatures.cpp
index cd00650db55cc9..f27f180d71a8ae 100644
--- a/clang/test/AST/ast-dump-fpfeatures.cpp
+++ b/clang/test/AST/ast-dump-fpfeatures.cpp
@@ -248,4 +248,16 @@ __attribute__((optnone)) T func_22(T x, T y) {
float func_23(float x, float y) {
return func_22(x, y);
-}
\ No newline at end of file
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} func_24 'vector2float (vector2double)'
+// CHECK: CompoundStmt {{.*}} FPContractMode=2 ConstRoundingMode=towardzero
+// CHECK: ReturnStmt
+// CHECK: ConvertVectorExpr {{.*}} FPContractMode=2 ConstRoundingMode=towardzero
+
+typedef double vector2double __attribute__((__vector_size__(16)));
+typedef float vector2float __attribute__((__vector_size__(8)));
+#pragma STDC FENV_ROUND FE_TOWARDZERO
+vector2float func_24(vector2double x) {
+ return __builtin_convertvector(x, vector2float);
+}
diff --git a/clang/test/CodeGen/pragma-fenv_access.c b/clang/test/CodeGen/pragma-fenv_access.c
index afca115ed08d1c..347e9670c47426 100644
--- a/clang/test/CodeGen/pragma-fenv_access.c
+++ b/clang/test/CodeGen/pragma-fenv_access.c
@@ -242,3 +242,12 @@ float func_20(float x, float y) {
// CHECK-LABEL: @func_20
// STRICT: call float @llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
// DEFAULT: fadd float
+
+typedef double vector4double __attribute__((__vector_size__(32)));
+typedef float vector4float __attribute__((__vector_size__(16)));
+vector4float func_21(vector4double x) {
+ #pragma STDC FENV_ROUND FE_UPWARD
+ return __builtin_convertvector(x, vector4float);
+}
+// CHECK-LABEL: @func_21
+// STRICT: call <4 x float> @llvm.experimental.constrained.fptrunc.v4f32.v4f64(<4 x double> {{.*}}, metadata !"round.upward", metadata !"fpexcept.strict")
|
erichkeane
reviewed
Feb 3, 2025
There was a problem hiding this comment.
1 nit, otherwise no real concerns. I don't know enough about this feature to be great at reviewing, and the nit might be just a fallout from what we already have, but otherwise it seems reasonable enough to me.
Glad to see we have a ast-dump test that seems to have tests for serialization, though wouldn't mind a little bit more of a 'small test' on it to ensure we are printing everything.
shafik
reviewed
Feb 3, 2025
| typedef double vector2double __attribute__((__vector_size__(16))); | ||
| typedef float vector2float __attribute__((__vector_size__(8))); | ||
| #pragma STDC FENV_ROUND FE_TOWARDZERO |
There was a problem hiding this comment.
I don't see other tests doing this but we should test also the absence of FP options in the AST as well when nothing is set. This would test that we are not incorrectly identifying that hasStoredFPFeatures() is true and that we don't get garbage values.
There was a problem hiding this comment.
added in f517b49. I moved it to the beginning of the file due to FP pragmas outside function scopes.
shafik
reviewed
Feb 3, 2025
This patch allows using fpfeatures pragmas with __builtin_convertvector: - added TrailingObjects with FPOptionsOverride and methods for handling it to ConvertVectorExpr - added support for codegen, node dumping, and serialization of fpfeatures contained in ConvertVectorExpr
ficol
force-pushed
the
convertvector-fpfeatures
branch
from
481eec7 to
f517b49
Compare
|
Are fp options (already) handled in constant evaluation? |
I think they are, there are tests for it in https://github.com/llvm/llvm-project/blob/main/clang/test/AST/const-fpfeatures.c and https://github.com/llvm/llvm-project/blob/main/clang/test/AST/const-fpfeatures.cpp |
They don't seem to test vectors though, does |
It seems to work. I tested it with: and it produces: Although, it feels a little bit weird that e and f evaluate to different values but it behaves the same way for h and i with scalar cast. |
|
Hi @shafik, can you please review this PR after changes? |
tbaederr
approved these changes
Feb 16, 2025
|
@erichkeane, @shafik can you help with merging this PR? |
|
@ficol Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch allows using fpfeatures pragmas with __builtin_convertvector: