[clang] c49cde6 - [Sema] Fix `ExtVectorElementExpr` tree transform for the `isArrow` case.
Author: Michele Scandale
Date: 2022-10-11T13:29:20-07:00
New Revision: c49cde6467f9bf200640db763152a9dc7f009520
URL:
https://github.com/llvm/llvm-project/commit/c49cde6467f9bf200640db763152a9dc7f009520
DIFF:
https://github.com/llvm/llvm-project/commit/c49cde6467f9bf200640db763152a9dc7f009520.diff
LOG: [Sema] Fix `ExtVectorElementExpr` tree transform for the `isArrow` case.
Make sure we propagate the value for `IsArrow` to
`RebuildExtVectorElementExpr` in order to be able to handle correctly
vector component accesses where the base value is a pointer.
Reviewed By: rjmccall
Differential Revision: https://reviews.llvm.org/D131698
Added:
Modified:
clang/lib/Sema/TreeTransform.h
clang/test/SemaTemplate/instantiate-clang.cpp
Removed:
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 82b6cfacf46b6..d772d7f69d80f 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -2814,20 +2814,18 @@ class TreeTransform {
///
/// By default, performs semantic analysis to build the new expression.
/// Subclasses may override this routine to provide
diff erent behavior.
- ExprResult RebuildExtVectorElementExpr(Expr *Base,
- SourceLocation OpLoc,
- SourceLocation AccessorLoc,
- IdentifierInfo &Accessor) {
+ ExprResult RebuildExtVectorElementExpr(Expr *Base, SourceLocation OpLoc,
+ bool IsArrow,
+ SourceLocation AccessorLoc,
+ IdentifierInfo &Accessor) {
CXXScopeSpec SS;
DeclarationNameInfo NameInfo(&Accessor, AccessorLoc);
-return getSema().BuildMemberReferenceExpr(Base, Base->getType(),
- OpLoc, /*IsArrow*/ false,
- SS, SourceLocation(),
- /*FirstQualifierInScope*/
nullptr,
- NameInfo,
- /* TemplateArgs */ nullptr,
- /*S*/ nullptr);
+return getSema().BuildMemberReferenceExpr(
+Base, Base->getType(), OpLoc, IsArrow, SS, SourceLocation(),
+/*FirstQualifierInScope*/ nullptr, NameInfo,
+/* TemplateArgs */ nullptr,
+/*S*/ nullptr);
}
/// Build a new initializer list expression.
@@ -11424,9 +11422,9 @@
TreeTransform::TransformExtVectorElementExpr(ExtVectorElementExpr *E) {
// FIXME: Bad source location
SourceLocation FakeOperatorLoc =
SemaRef.getLocForEndOfToken(E->getBase()->getEndLoc());
- return getDerived().RebuildExtVectorElementExpr(Base.get(), FakeOperatorLoc,
- E->getAccessorLoc(),
- E->getAccessor());
+ return getDerived().RebuildExtVectorElementExpr(
+ Base.get(), FakeOperatorLoc, E->isArrow(), E->getAccessorLoc(),
+ E->getAccessor());
}
template
diff --git a/clang/test/SemaTemplate/instantiate-clang.cpp
b/clang/test/SemaTemplate/instantiate-clang.cpp
index 34d68c4e59fd5..18b428834b0a2 100644
--- a/clang/test/SemaTemplate/instantiate-clang.cpp
+++ b/clang/test/SemaTemplate/instantiate-clang.cpp
@@ -18,7 +18,15 @@ struct ExtVectorAccess0 {
template struct ExtVectorAccess0;
template struct ExtVectorAccess0;
-typedef __attribute__(( ext_vector_type(2) )) double double2;
+template
+struct ExtVectorAccess1 {
+ void f(T *v1, double4 *v2) {
+v1->xy = v2->yx;
+ }
+};
+
+template struct ExtVectorAccess1;
+template struct ExtVectorAccess1;
template
struct ShuffleVector0 {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] b7d7c44 - Fix `unsafe-fp-math` attribute emission.
Author: Michele Scandale
Date: 2022-11-14T20:40:57-08:00
New Revision: b7d7c448df9a4679c1dfa079911f863e32d8e41f
URL:
https://github.com/llvm/llvm-project/commit/b7d7c448df9a4679c1dfa079911f863e32d8e41f
DIFF:
https://github.com/llvm/llvm-project/commit/b7d7c448df9a4679c1dfa079911f863e32d8e41f.diff
LOG: Fix `unsafe-fp-math` attribute emission.
The conditions for which Clang emits the `unsafe-fp-math` function
attribute has been modified as part of
`84a9ec2ff1ee97fd7e8ed988f5e7b197aab84a7`.
In the backend code generators `"unsafe-fp-math"="true"` enable floating
point contraction for the whole function.
The intent of the change in `84a9ec2ff1ee97fd7e8ed988f5e7b197aab84a7`
was to prevent backend code generators performing contractions when that
is not expected.
However the change is inaccurate and incomplete because it allows
`unsafe-fp-math` to be set also when only in-statement contraction is
allowed.
Consider the following example
```
float foo(float a, float b, float c) {
float tmp = a * b;
return tmp + c;
}
```
and compile it with the command line
```
clang -fno-math-errno -funsafe-math-optimizations -ffp-contract=on \
-O2 -mavx512f -S -o -
```
The resulting assembly has a `vfmadd213ss` instruction which corresponds
to a fused multiply-add. From the user perspective there shouldn't be
any contraction because the multiplication and the addition are not in
the same statement.
The optimized IR is:
```
define float @test(float noundef %a, float noundef %b, float noundef %c) #0 {
%mul = fmul reassoc nsz arcp afn float %b, %a
%add = fadd reassoc nsz arcp afn float %mul, %c
ret float %add
}
attributes #0 = {
[...]
"no-signed-zeros-fp-math"="true"
"no-trapping-math"="true"
[...]
"unsafe-fp-math"="true"
}
```
The `"unsafe-fp-math"="true"` function attribute allows the backend code
generator to perform `(fadd (fmul a, b), c) -> (fmadd a, b, c)`.
In the current IR representation there is no way to determine the
statement boundaries from the original source code.
Because of this for in-statement only contraction the generated IR
doesn't have instructions with the `contract` fast-math flag and
`llvm.fmuladd` is being used to represent contractions opportunities
that occur within a single statement.
Therefore `"unsafe-fp-math"="true"` can only be emitted when contraction
across statements is allowed.
Moreover the change in `84a9ec2ff1ee97fd7e8ed988f5e7b197aab84a7` doesn't
take into account that the floating point math function attributes can
be refined during IR code generation of a function to handle the cases
where the floating point math options are modified within a compound
statement via pragmas (see `CGFPOptionsRAII`).
For consistency `unsafe-fp-math` needs to be disabled if the contraction
mode for any scope/operation is not `fast`.
Similarly for consistency reason the initialization of `UnsafeFPMath` of
in `TargetOptions` for the backend code generation should take into
account the contraction mode as well.
Reviewed By: zahiraam
Differential Revision: https://reviews.llvm.org/D136786
Added:
Modified:
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
clang/test/CodeGen/fp-function-attrs.cpp
clang/test/CodeGen/func-attr.c
Removed:
diff --git a/clang/lib/CodeGen/BackendUtil.cpp
b/clang/lib/CodeGen/BackendUtil.cpp
index 41f9ce3a009d..8498ca2e6338 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -409,7 +409,12 @@ static bool initTargetOptions(DiagnosticsEngine &Diags,
Options.NoInfsFPMath = LangOpts.NoHonorInfs;
Options.NoNaNsFPMath = LangOpts.NoHonorNaNs;
Options.NoZerosInBSS = CodeGenOpts.NoZeroInitializedInBSS;
- Options.UnsafeFPMath = LangOpts.UnsafeFPMath;
+ Options.UnsafeFPMath = LangOpts.AllowFPReassoc && LangOpts.AllowRecip &&
+ LangOpts.NoSignedZero && LangOpts.ApproxFunc &&
+ (LangOpts.getDefaultFPContractMode() ==
+ LangOptions::FPModeKind::FPM_Fast ||
+ LangOpts.getDefaultFPContractMode() ==
+ LangOptions::FPModeKind::FPM_FastHonorPragmas);
Options.ApproxFuncFPMath = LangOpts.ApproxFunc;
Options.BBSections =
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index b7d6ea2d3cc6..4b9c29c68181 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -1862,11 +1862,12 @@ void
CodeGenModule::getDefaultFunctionAttributes(StringRef Name,
FuncAttrs.addAttribute("no-nans-fp-math", "true");
if (LangOpts.ApproxFunc)
FuncAttrs.addAttribute("approx-func-fp-math", "true");
-if ((LangOpts.FastMath ||
- (!LangOpts.FastMath && LangOpts.AllowFPReassoc &&
- LangOpts.AllowRecip && !LangOpts.FiniteMathOnly &&
- LangOpts.NoSignedZero && Lan
[clang] [CodeGen] Do not emit call site attributes for LLVM intrinsics. (PR #116881)
https://github.com/michele-scandale created
https://github.com/llvm/llvm-project/pull/116881
When a function is declared with the `asm(...)` attribute to change the mangled
name to reference a LLVM intrinsic, the AST->IR translation for the function
declaration already skips any function/parameter attribute that is either
deduced from the AST function declaration or implied by language options/target
properties. Instead the attributes associated to the LLVM intrinsic function
are being generated.
When emitting calls to such function declarations, however, call site
attributes are emitted based on the AST function declaration or language
options/target properties.
This can lead to undesired outcomes, e.g. the call site of a LLVM intrinsic
marked `convergent` when the language options implies `convergent` by default.
This commit fixes the call lowering logic to ignore all attributes in the case
the generate call instruction is an intrinsic call, such that the only
attributes from the intrinsic declaration will be implicitly considered.
>From 889c42be07fba9c72238db77e3a36994946f44e2 Mon Sep 17 00:00:00 2001
From: Michele Scandale
Date: Tue, 19 Nov 2024 14:30:28 -0800
Subject: [PATCH] [CodeGen] Do not emit call site attributes for LLVM
intrinsics.
When a function is declared with the `asm(...)` attribute to change the
mangled name to reference a LLVM intrinsic, the AST->IR translation for
the function declaration already skips any function/parameter attribute
that is either deduced from the AST function declaration or implied by
language options/target properties. Instead the attributes associated
to the LLVM intrinsic function are being generated.
When emitting calls to such function declarations, however, call site
attributes are emitted based on the AST function declaration or language
options/target properties.
This can lead to undesired outcomes, e.g. the call site of a LLVM
intrinsic marked `convergent` when the language options implies
`convergent` by default.
This commit fixes the call lowering logic to ignore all attributes in
the case the generate call instruction is an intrinsic call, such that
the only attributes from the intrinsic declaration will be implicitly
considered.
---
clang/lib/CodeGen/CGCall.cpp | 10 --
clang/test/CodeGen/llvm-intrinsic-call.c | 13 +
clang/test/CodeGenCUDA/surface.cu| 2 +-
3 files changed, 22 insertions(+), 3 deletions(-)
create mode 100644 clang/test/CodeGen/llvm-intrinsic-call.c
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 35d495d4dfab82..7af3c676182e5f 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5793,8 +5793,14 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo
&CallInfo,
}
// Apply the attributes and calling convention.
- CI->setAttributes(Attrs);
- CI->setCallingConv(static_cast(CallingConv));
+
+ // If this is a call to an intrinsic, ignore the attributes that would
+ // normally be deduced from the AST function declaration + the default
+ // attributes imposed by the language and/or target.
+ if (!isa(CI)) {
+CI->setAttributes(Attrs);
+CI->setCallingConv(static_cast(CallingConv));
+ }
// Apply various metadata.
diff --git a/clang/test/CodeGen/llvm-intrinsic-call.c
b/clang/test/CodeGen/llvm-intrinsic-call.c
new file mode 100644
index 00..f71769d652765e
--- /dev/null
+++ b/clang/test/CodeGen/llvm-intrinsic-call.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
+
+float llvm_sin_f32(float) asm("llvm.sin.f32");
+
+float test(float a) {
+ return llvm_sin_f32(a);
+}
+
+// CHECK: call float @llvm.sin.f32(float {{%.+}}){{$}}
+
+// CHECK: declare float @llvm.sin.f32(float) [[ATTRS:#[0-9]+]]
+
+// CHECK: attributes [[ATTRS]] = { nocallback nofree nosync nounwind
speculatable willreturn memory(none) }
diff --git a/clang/test/CodeGenCUDA/surface.cu
b/clang/test/CodeGenCUDA/surface.cu
index 4106673f3138af..c9e9f1543ec7df 100644
--- a/clang/test/CodeGenCUDA/surface.cu
+++ b/clang/test/CodeGenCUDA/surface.cu
@@ -29,7 +29,7 @@ __attribute__((device)) int suld_2d_zero(surface,
int, int) asm("llvm.n
// DEVICE-LABEL: i32 @_Z3fooii(i32 noundef %x, i32 noundef %y)
// DEVICE: call i64 @llvm.nvvm.texsurf.handle.internal.p1(ptr addrspace(1)
@surf)
-// DEVICE: call noundef i32 @llvm.nvvm.suld.2d.i32.zero(i64 %{{.*}}, i32
noundef %{{.*}}, i32 noundef %{{.*}})
+// DEVICE: call i32 @llvm.nvvm.suld.2d.i32.zero(i64 %{{.*}}, i32 %{{.*}}, i32
%{{.*}})
__attribute__((device)) int foo(int x, int y) {
return suld_2d_zero(surf, x, y);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CodeGen] Do not emit call site attributes for LLVM intrinsics. (PR #116881)
https://github.com/michele-scandale closed https://github.com/llvm/llvm-project/pull/116881 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CodeGen] Do not emit call site attributes for LLVM intrinsics. (PR #116881)
michele-scandale wrote: > Anything we do to make this work better risks being interpreted as treating > this as somehow supported, which it absolutely is not. Alright. I guess I'll close this PR. https://github.com/llvm/llvm-project/pull/116881 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CodeGen] Do not emit call site attributes for LLVM intrinsics. (PR #116881)
michele-scandale wrote:
I understand this is not something recommended.
Still there is already code like the one in
https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CodeGenModule.cpp#L2887-L2892
```
void CodeGenModule::SetFunctionAttributes(GlobalDecl GD, llvm::Function *F,
bool IsIncompleteFunction,
bool IsThunk) {
if (llvm::Intrinsic::ID IID = F->getIntrinsicID()) {
// If this is an intrinsic function, set the function's attributes
// to the intrinsic's attributes.
F->setAttributes(llvm::Intrinsic::getAttributes(getLLVMContext(), IID));
return;
}
```
which a small trace of intention to do something special about this case.
However this code is also quite old (2011) so it might still be from a time
where there was a different idea about this.
https://github.com/llvm/llvm-project/pull/116881
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Rework `hasBooleanRepresentation`. (PR #136038)
@@ -8623,6 +8624,13 @@ inline bool Type::isIntegralOrEnumerationType() const {
inline bool Type::isBooleanType() const {
if (const auto *BT = dyn_cast(CanonicalType))
return BT->getKind() == BuiltinType::Bool;
+ if (const EnumType *ET = dyn_cast(CanonicalType)) {
+// Incomplete enum types are not treated as integer types.
+// FIXME: In C++, enum types are never integer types.
+return IsEnumDeclComplete(ET->getDecl()) &&
+ !IsEnumDeclScoped(ET->getDecl()) &&
michele-scandale wrote:
> C specifies that "enumerated types" are integer types, and C++ does not, and
> `isIntegerType` is trying to honor that as best it can. It's one of those
> situations where we need to ask what callers actually want, because in most
> cases it's probably not this language-specific formal property. If
> `hasIntegerRepresentation` is really a question about the representation of
> the type, it should be ignoring scoped-ness of enums. But it seems to be used
> in Sema in ways that should definitely exclude scoped enums, like the
> semantic analysis of `__builtin_shuffle_vector` and vector math.
The check on scoped enum comes from:
```
commit 21673c4e7ec08457b53798b9879b7cc9a5909eb8
Author: Douglas Gregor
Date: Thu May 5 16:13:52 2011 +
Scoped enumerations should not be treated as integer types (in the C
sense). Fixes by eliminating an inconsistency
between C++ overloading (which handled scoped enumerations correctly)
and C binary operator type-checking (which didn't).
llvm-svn: 130924
```
> `isBooleanType` should probably continue to check only for `bool`.
Ok.
> It seems reasonable for `hasBooleanRepresentation` to look through enums,
> though, and it should probably ignore the scoped-ness of the enum.
I can do that, but I'm not super happy about the inconsistency.
https://github.com/llvm/llvm-project/pull/136038
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Rework `hasBooleanRepresentation`. (PR #136038)
@@ -8623,6 +8624,13 @@ inline bool Type::isIntegralOrEnumerationType() const {
inline bool Type::isBooleanType() const {
if (const auto *BT = dyn_cast(CanonicalType))
return BT->getKind() == BuiltinType::Bool;
+ if (const EnumType *ET = dyn_cast(CanonicalType)) {
+// Incomplete enum types are not treated as integer types.
+// FIXME: In C++, enum types are never integer types.
+return IsEnumDeclComplete(ET->getDecl()) &&
+ !IsEnumDeclScoped(ET->getDecl()) &&
michele-scandale wrote:
>Since the existing methods are arguably misnamed, I wonder if we could
>reasonably just change them. Since the difference is (AFAIK) purely to make
>them vector-inclusive, perhaps we should just remove them and have a
>getNonVectorType() method that clients are expected to call before using the
>existing predicates.
Just to make sure I'm not misunderstanding things: are you suggesting to just
drop the "vector of" portion of the `has*Representation` and move that onto
clients of these APIs, right?
I'm not sure if that can work in general. E.g.
```
bool Type::hasIntegerRepresentation() const {
if (const auto *VT = dyn_cast(CanonicalType))
return VT->getElementType()->isIntegerType();
if (CanonicalType->isSveVLSBuiltinType()) {
const auto *VT = cast(CanonicalType);
return VT->getKind() == BuiltinType::SveBool ||
(VT->getKind() >= BuiltinType::SveInt8 &&
VT->getKind() <= BuiltinType::SveUint64);
}
if (CanonicalType->isRVVVLSBuiltinType()) {
const auto *VT = cast(CanonicalType);
return (VT->getKind() >= BuiltinType::RvvInt8mf8 &&
VT->getKind() <= BuiltinType::RvvUint64m8);
}
return isIntegerType();
}
```
This is more than just types where `isIntegerType` is `true`, or vector of such
types.
https://github.com/llvm/llvm-project/pull/136038
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Rework `hasBooleanRepresentation`. (PR #136038)
michele-scandale wrote: Putting aside for now the naming issue (https://github.com/llvm/llvm-project/pull/136038#discussion_r2049703902), is there anything left for the changes proposed in this PR? https://github.com/llvm/llvm-project/pull/136038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Rework `hasBooleanRepresentation`. (PR #136038)
@@ -8623,6 +8624,13 @@ inline bool Type::isIntegralOrEnumerationType() const {
inline bool Type::isBooleanType() const {
if (const auto *BT = dyn_cast(CanonicalType))
return BT->getKind() == BuiltinType::Bool;
+ if (const EnumType *ET = dyn_cast(CanonicalType)) {
+// Incomplete enum types are not treated as integer types.
+// FIXME: In C++, enum types are never integer types.
+return IsEnumDeclComplete(ET->getDecl()) &&
+ !IsEnumDeclScoped(ET->getDecl()) &&
michele-scandale wrote:
This is just the same code that exist in `isIntegerType`. I am puzzled by this
code as well, but I suppose there is a good reason for it.
https://github.com/llvm/llvm-project/pull/136038
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Rework `hasBooleanRepresentation`. (PR #136038)
@@ -8623,6 +8624,13 @@ inline bool Type::isIntegralOrEnumerationType() const {
inline bool Type::isBooleanType() const {
if (const auto *BT = dyn_cast(CanonicalType))
return BT->getKind() == BuiltinType::Bool;
+ if (const EnumType *ET = dyn_cast(CanonicalType)) {
michele-scandale wrote:
This is to match `isIntegerType` that is used by `hasIntegerRepresentation`.
I'm not entirely sure it makes sense to handle enums here (given also the FIXME
comment).
I'll try to see what breaks by removing this.
https://github.com/llvm/llvm-project/pull/136038
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Rework `hasBooleanRepresentation`. (PR #136038)
@@ -2336,16 +2336,9 @@ bool Type::isArithmeticType() const {
}
bool Type::hasBooleanRepresentation() const {
- if (isBooleanType())
-return true;
-
- if (const EnumType *ET = getAs())
-return ET->getDecl()->getIntegerType()->isBooleanType();
-
- if (const AtomicType *AT = getAs())
-return AT->getValueType()->hasBooleanRepresentation();
-
- return false;
+ if (const auto *VT = dyn_cast(CanonicalType))
michele-scandale wrote:
I don't think referring to the how the type is lowered in IR make sense for a
function in `Type`.
The overall question should be about this family of functions:
```
/// Determine whether this type has an integer representation
/// of some sort, e.g., it is an integer type or a vector.
bool hasIntegerRepresentation() const;
/// Determine whether this type has an signed integer representation
/// of some sort, e.g., it is an signed integer type or a vector.
bool hasSignedIntegerRepresentation() const;
/// Determine whether this type has an unsigned integer representation
/// of some sort, e.g., it is an unsigned integer type or a vector.
bool hasUnsignedIntegerRepresentation() const;
/// Determine whether this type has a floating-point representation
/// of some sort, e.g., it is a floating-point type or a vector thereof.
bool hasFloatingRepresentation() const;
```
In here I'm just trying to have `hasBooleanRepresentation` be consistent with
those.
https://github.com/llvm/llvm-project/pull/136038
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Rework `hasBooleanRepresentation`. (PR #136038)
https://github.com/michele-scandale updated
https://github.com/llvm/llvm-project/pull/136038
>From 17e73775bf3acdece21c5bcd6ce6b5340e5c6f12 Mon Sep 17 00:00:00 2001
From: Michele Scandale
Date: Thu, 17 Apr 2025 13:30:04 -0700
Subject: [PATCH] [clang] Rework `hasBooleanRepresentation`.
This is a follow-up of 13aac46332f607a38067b5ddd466071683b8c255.
This commit adjusts the implementation of `hasBooleanRepresentation` to
somewhat aligned to `hasIntegerRepresentation`.
In particular vector of booleans should be handled in
`hasBooleanRepresentation`, while `_Atomic(bool)` should not.
---
clang/include/clang/AST/Type.h | 5 +++--
clang/lib/AST/Type.cpp | 17 +++--
clang/lib/CodeGen/CGExpr.cpp | 20 +---
3 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 1ecd64539e2de..c3c2fe6249526 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -2773,8 +2773,9 @@ class alignas(TypeAlignment) Type : public
ExtQualsTypeCommonBase {
/// of some sort, e.g., it is a floating-point type or a vector thereof.
bool hasFloatingRepresentation() const;
- /// Determine whether this type has a boolean representation
- /// of some sort.
+ /// Determine whether this type has a boolean representation -- i.e., it is a
+ /// boolean type, an enum type whose underlying type is a boolean type, or a
+ /// vector of booleans.
bool hasBooleanRepresentation() const;
// Type Checking Functions: Check to see if this type is structurally the
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 42e94d66d1a13..bf659843ed35c 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -2344,16 +2344,13 @@ bool Type::isArithmeticType() const {
}
bool Type::hasBooleanRepresentation() const {
- if (isBooleanType())
-return true;
-
- if (const EnumType *ET = getAs())
-return ET->getDecl()->getIntegerType()->isBooleanType();
-
- if (const AtomicType *AT = getAs())
-return AT->getValueType()->hasBooleanRepresentation();
-
- return false;
+ if (const auto *VT = dyn_cast(CanonicalType))
+return VT->getElementType()->isBooleanType();
+ if (const auto *ET = dyn_cast(CanonicalType)) {
+return ET->getDecl()->isComplete() &&
+ ET->getDecl()->getIntegerType()->isBooleanType();
+ }
+ return isBooleanType();
}
Type::ScalarTypeKind Type::getScalarTypeKind() const {
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 1efc626a8f5a6..b9d8266576de4 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -1920,7 +1920,7 @@ static bool getRangeForType(CodeGenFunction &CGF,
QualType Ty,
llvm::MDNode *CodeGenFunction::getRangeForLoadFromType(QualType Ty) {
llvm::APInt Min, End;
if (!getRangeForType(*this, Ty, Min, End, CGM.getCodeGenOpts().StrictEnums,
- Ty->hasBooleanRepresentation()))
+ Ty->hasBooleanRepresentation() && !Ty->isVectorType()))
return nullptr;
llvm::MDBuilder MDHelper(getLLVMContext());
@@ -1948,7 +1948,7 @@ bool CodeGenFunction::EmitScalarRangeCheck(llvm::Value
*Value, QualType Ty,
if (!HasBoolCheck && !HasEnumCheck)
return false;
- bool IsBool = Ty->hasBooleanRepresentation() ||
+ bool IsBool = (Ty->hasBooleanRepresentation() && !Ty->isVectorType()) ||
NSAPI(CGM.getContext()).isObjCBOOLType(Ty);
bool NeedsBoolCheck = HasBoolCheck && IsBool;
bool NeedsEnumCheck = HasEnumCheck && Ty->getAs();
@@ -2068,11 +2068,8 @@ llvm::Value *CodeGenFunction::EmitLoadOfScalar(Address
Addr, bool Volatile,
/// by ConvertType) to its load/store type (as returned by
/// convertTypeForLoadStore).
llvm::Value *CodeGenFunction::EmitToMemory(llvm::Value *Value, QualType Ty) {
- if (Ty->hasBooleanRepresentation() || Ty->isBitIntType()) {
-llvm::Type *StoreTy = convertTypeForLoadStore(Ty, Value->getType());
-bool Signed = Ty->isSignedIntegerOrEnumerationType();
-return Builder.CreateIntCast(Value, StoreTy, Signed, "storedv");
- }
+ if (auto *AtomicTy = Ty->getAs())
+Ty = AtomicTy->getValueType();
if (Ty->isExtVectorBoolType()) {
llvm::Type *StoreTy = convertTypeForLoadStore(Ty, Value->getType());
@@ -2088,6 +2085,12 @@ llvm::Value *CodeGenFunction::EmitToMemory(llvm::Value
*Value, QualType Ty) {
Value = Builder.CreateBitCast(Value, StoreTy);
}
+ if (Ty->hasBooleanRepresentation() || Ty->isBitIntType()) {
+llvm::Type *StoreTy = convertTypeForLoadStore(Ty, Value->getType());
+bool Signed = Ty->isSignedIntegerOrEnumerationType();
+return Builder.CreateIntCast(Value, StoreTy, Signed, "storedv");
+ }
+
return Value;
}
@@ -2095,6 +2098,9 @@ llvm::Value *CodeGenFunction::EmitToMemory(llvm::Value
*Value, QualType Ty) {
/// by convertTypeForLoadStore) to its primary IR type (as returned
/// by ConvertType).
llvm::Value *CodeGenFunction::
[clang] [clang] Rework `hasBooleanRepresentation`. (PR #136038)
https://github.com/michele-scandale closed https://github.com/llvm/llvm-project/pull/136038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Refactor CodeGen's hasBooleanRepresentation (PR #134159)
michele-scandale wrote:
Given that there are already similar functions in the `Type` class -- e.g.
`has{Signed,Unsigned,}IntegerRepresentation`, `hasFloatingRepresentation` -- it
seems a bit inconsistent the definition of `hasBooleanRepresentation`. I would
expect that "vector of booleans" to have `hasBooleanRepresentation` returning
`true`, and I would expect `_Atomic()` to
have `hasBooleanRepresentation` returning `false`.
Any thoughts?
https://github.com/llvm/llvm-project/pull/134159
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Rework `hasBooleanRepresentation`. (PR #136038)
@@ -8623,6 +8624,13 @@ inline bool Type::isIntegralOrEnumerationType() const {
inline bool Type::isBooleanType() const {
if (const auto *BT = dyn_cast(CanonicalType))
return BT->getKind() == BuiltinType::Bool;
+ if (const EnumType *ET = dyn_cast(CanonicalType)) {
michele-scandale wrote:
>From a local run of `check-clang` there are no tests failing with this part of
>the code.
https://github.com/llvm/llvm-project/pull/136038
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Rework `hasBooleanRepresentation`. (PR #136038)
https://github.com/michele-scandale edited https://github.com/llvm/llvm-project/pull/136038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Refactor CodeGen's hasBooleanRepresentation (PR #134159)
michele-scandale wrote: > There is already a precedent (hasPointerRepresentation) for not including > vectors You cannot have vectors of pointers in the C/C++ extensions for vector types. > Also, vectors of Booleans have different considerations that scalar Booleans. > For instance, they are stored differently in memory. The fact that the type is packed in memory does not change the fact that they represent boolean values. > Maybe the function should have a different name to avoid confusion? That would help to remove the confusion, but I'm not sure what could be a better name other than `isBoolOrEnumBoolOrAtomicBool`. In parallel I'm trying to rework the function to have the same structure as `hasIntegerRepresentation`. I'll soon post a PR to see if that is acceptable. If not, then I think it would be better to rename `hasBooleanRepresentation`. https://github.com/llvm/llvm-project/pull/134159 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Refactor CodeGen's hasBooleanRepresentation (PR #134159)
michele-scandale wrote: See https://github.com/llvm/llvm-project/pull/136038 https://github.com/llvm/llvm-project/pull/134159 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Rework `hasBooleanRepresentation`. (PR #136038)
https://github.com/michele-scandale created
https://github.com/llvm/llvm-project/pull/136038
This is a follow-up of 13aac46332f607a38067b5ddd466071683b8c255. This commit
adjusts the implementation of `hasBooleanRepresentation` to have a similar
behavior as the one of `hasIntegerRepresentation`. In particular vector of
booleans should be handled in `hasBooleanRepresentation`, while `_Atomic(bool)`
should not.
>From 31777c1aad43b3dfc214beefe41c170f86aa9f04 Mon Sep 17 00:00:00 2001
From: Michele Scandale
Date: Wed, 16 Apr 2025 14:41:59 -0700
Subject: [PATCH] [clang] Rework `hasBooleanRepresentation`.
This is a follow-up of 13aac46332f607a38067b5ddd466071683b8c255.
This commit adjusts the implementation of `hasBooleanRepresentation` to
have a similar behavior as the one of `hasIntegerRepresentation`.
In particular vector of booleans should be handled in
`hasBooleanRepresentation`, while `_Atomic(bool)` should not.
---
clang/include/clang/AST/Decl.h | 8
clang/include/clang/AST/Type.h | 10 +-
clang/lib/AST/Type.cpp | 13 +++--
clang/lib/CodeGen/CGExpr.cpp | 20 +---
clang/lib/Sema/SemaType.cpp| 2 +-
5 files changed, 34 insertions(+), 19 deletions(-)
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 3faf63e395a08..07ada202075a2 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -5238,6 +5238,14 @@ inline bool IsEnumDeclScoped(EnumDecl *ED) {
return ED->isScoped();
}
+/// Return the integer type corresponding to the given decl.
+///
+/// We use this function to break a cycle between the inline definitions in
+/// Type.h and Decl.h.
+inline QualType GetEnumDeclIntegerType(EnumDecl *ED) {
+ return ED->getIntegerType();
+}
+
/// OpenMP variants are mangled early based on their OpenMP context selector.
/// The new name looks likes this:
/// + OpenMPVariantManglingSeparatorStr +
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 5bf036e3347eb..ce6904abcc71a 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -2774,7 +2774,7 @@ class alignas(TypeAlignment) Type : public
ExtQualsTypeCommonBase {
bool hasFloatingRepresentation() const;
/// Determine whether this type has a boolean representation
- /// of some sort.
+ /// of some sort, e.g., it is a boolean type or a vector thereof.
bool hasBooleanRepresentation() const;
// Type Checking Functions: Check to see if this type is structurally the
@@ -8531,6 +8531,7 @@ inline bool Type::isNullPtrType() const {
bool IsEnumDeclComplete(EnumDecl *);
bool IsEnumDeclScoped(EnumDecl *);
+QualType GetEnumDeclIntegerType(EnumDecl *);
inline bool Type::isIntegerType() const {
if (const auto *BT = dyn_cast(CanonicalType))
@@ -8623,6 +8624,13 @@ inline bool Type::isIntegralOrEnumerationType() const {
inline bool Type::isBooleanType() const {
if (const auto *BT = dyn_cast(CanonicalType))
return BT->getKind() == BuiltinType::Bool;
+ if (const EnumType *ET = dyn_cast(CanonicalType)) {
+// Incomplete enum types are not treated as integer types.
+// FIXME: In C++, enum types are never integer types.
+return IsEnumDeclComplete(ET->getDecl()) &&
+ !IsEnumDeclScoped(ET->getDecl()) &&
+ GetEnumDeclIntegerType(ET->getDecl())->isBooleanType();
+ }
return false;
}
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 53620003c9655..b456f43d39224 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -2336,16 +2336,9 @@ bool Type::isArithmeticType() const {
}
bool Type::hasBooleanRepresentation() const {
- if (isBooleanType())
-return true;
-
- if (const EnumType *ET = getAs())
-return ET->getDecl()->getIntegerType()->isBooleanType();
-
- if (const AtomicType *AT = getAs())
-return AT->getValueType()->hasBooleanRepresentation();
-
- return false;
+ if (const auto *VT = dyn_cast(CanonicalType))
+return VT->getElementType()->isBooleanType();
+ return isBooleanType();
}
Type::ScalarTypeKind Type::getScalarTypeKind() const {
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index abb88477062fc..786a56eed7ed5 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -1920,7 +1920,7 @@ static bool getRangeForType(CodeGenFunction &CGF,
QualType Ty,
llvm::MDNode *CodeGenFunction::getRangeForLoadFromType(QualType Ty) {
llvm::APInt Min, End;
if (!getRangeForType(*this, Ty, Min, End, CGM.getCodeGenOpts().StrictEnums,
- Ty->hasBooleanRepresentation()))
+ Ty->hasBooleanRepresentation() && !Ty->isVectorType()))
return nullptr;
llvm::MDBuilder MDHelper(getLLVMContext());
@@ -1948,7 +1948,7 @@ bool CodeGenFunction::EmitScalarRangeCheck(llvm::Value
*Value, QualType Ty,
if (!HasBoolCheck && !HasEnumCheck)
return false;
- bool IsBool = Ty->hasBooleanRepresenta
[clang] [clang] Rework `hasBooleanRepresentation`. (PR #136038)
@@ -2088,13 +2085,22 @@ llvm::Value *CodeGenFunction::EmitToMemory(llvm::Value
*Value, QualType Ty) {
Value = Builder.CreateBitCast(Value, StoreTy);
}
+ if (Ty->hasBooleanRepresentation() || Ty->isBitIntType()) {
michele-scandale wrote:
I'll double check. I know there have been changes recently to support a
different ABI in the HLSL case.
https://github.com/llvm/llvm-project/pull/136038
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Rework `hasBooleanRepresentation`. (PR #136038)
https://github.com/michele-scandale updated
https://github.com/llvm/llvm-project/pull/136038
>From 359e31eece5b7ddf4a62074a67947c31f4538ed1 Mon Sep 17 00:00:00 2001
From: Michele Scandale
Date: Mon, 21 Apr 2025 12:47:10 -0700
Subject: [PATCH] [clang] Rework `hasBooleanRepresentation`.
This is a follow-up of 13aac46332f607a38067b5ddd466071683b8c255.
This commit adjusts the implementation of `hasBooleanRepresentation` to
somewhat aligned to `hasIntegerRepresentation`.
In particular vector of booleans should be handled in
`hasBooleanRepresentation`, while `_Atomic(bool)` should not.
---
clang/include/clang/AST/Type.h | 5 +++--
clang/lib/AST/Type.cpp | 19 +--
clang/lib/CodeGen/CGExpr.cpp | 20 +---
3 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 1ecd64539e2de..c3c2fe6249526 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -2773,8 +2773,9 @@ class alignas(TypeAlignment) Type : public
ExtQualsTypeCommonBase {
/// of some sort, e.g., it is a floating-point type or a vector thereof.
bool hasFloatingRepresentation() const;
- /// Determine whether this type has a boolean representation
- /// of some sort.
+ /// Determine whether this type has a boolean representation -- i.e., it is a
+ /// boolean type, an enum type whose underlying type is a boolean type, or a
+ /// vector of booleans.
bool hasBooleanRepresentation() const;
// Type Checking Functions: Check to see if this type is structurally the
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 42e94d66d1a13..8a8549e46aa0c 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -2344,16 +2344,15 @@ bool Type::isArithmeticType() const {
}
bool Type::hasBooleanRepresentation() const {
- if (isBooleanType())
-return true;
-
- if (const EnumType *ET = getAs())
-return ET->getDecl()->getIntegerType()->isBooleanType();
-
- if (const AtomicType *AT = getAs())
-return AT->getValueType()->hasBooleanRepresentation();
-
- return false;
+ if (const auto *VT = dyn_cast(CanonicalType))
+return VT->getElementType()->isBooleanType();
+ if (const auto *ET = dyn_cast(CanonicalType)) {
+return ET->getDecl()->isComplete() &&
+ ET->getDecl()->getIntegerType()->isBooleanType();
+ }
+ if (const auto *IT = dyn_cast(CanonicalType))
+return IT->getNumBits() == 1;
+ return isBooleanType();
}
Type::ScalarTypeKind Type::getScalarTypeKind() const {
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 1efc626a8f5a6..b9d8266576de4 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -1920,7 +1920,7 @@ static bool getRangeForType(CodeGenFunction &CGF,
QualType Ty,
llvm::MDNode *CodeGenFunction::getRangeForLoadFromType(QualType Ty) {
llvm::APInt Min, End;
if (!getRangeForType(*this, Ty, Min, End, CGM.getCodeGenOpts().StrictEnums,
- Ty->hasBooleanRepresentation()))
+ Ty->hasBooleanRepresentation() && !Ty->isVectorType()))
return nullptr;
llvm::MDBuilder MDHelper(getLLVMContext());
@@ -1948,7 +1948,7 @@ bool CodeGenFunction::EmitScalarRangeCheck(llvm::Value
*Value, QualType Ty,
if (!HasBoolCheck && !HasEnumCheck)
return false;
- bool IsBool = Ty->hasBooleanRepresentation() ||
+ bool IsBool = (Ty->hasBooleanRepresentation() && !Ty->isVectorType()) ||
NSAPI(CGM.getContext()).isObjCBOOLType(Ty);
bool NeedsBoolCheck = HasBoolCheck && IsBool;
bool NeedsEnumCheck = HasEnumCheck && Ty->getAs();
@@ -2068,11 +2068,8 @@ llvm::Value *CodeGenFunction::EmitLoadOfScalar(Address
Addr, bool Volatile,
/// by ConvertType) to its load/store type (as returned by
/// convertTypeForLoadStore).
llvm::Value *CodeGenFunction::EmitToMemory(llvm::Value *Value, QualType Ty) {
- if (Ty->hasBooleanRepresentation() || Ty->isBitIntType()) {
-llvm::Type *StoreTy = convertTypeForLoadStore(Ty, Value->getType());
-bool Signed = Ty->isSignedIntegerOrEnumerationType();
-return Builder.CreateIntCast(Value, StoreTy, Signed, "storedv");
- }
+ if (auto *AtomicTy = Ty->getAs())
+Ty = AtomicTy->getValueType();
if (Ty->isExtVectorBoolType()) {
llvm::Type *StoreTy = convertTypeForLoadStore(Ty, Value->getType());
@@ -2088,6 +2085,12 @@ llvm::Value *CodeGenFunction::EmitToMemory(llvm::Value
*Value, QualType Ty) {
Value = Builder.CreateBitCast(Value, StoreTy);
}
+ if (Ty->hasBooleanRepresentation() || Ty->isBitIntType()) {
+llvm::Type *StoreTy = convertTypeForLoadStore(Ty, Value->getType());
+bool Signed = Ty->isSignedIntegerOrEnumerationType();
+return Builder.CreateIntCast(Value, StoreTy, Signed, "storedv");
+ }
+
return Value;
}
@@ -2095,6 +2098,9 @@ llvm::Value *CodeGenFunction::EmitToMemory(llvm::Value
*Value, QualType Ty) {
/// by convertTypeForLoadStore) to
