Skip to content

[clang] improve consistency with GCC vector comparison #148954

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+46 −25
Open

[clang] improve consistency with GCC vector comparison #148954

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3b19742
[clang] improve consistency with GCC vector comparison
ImpleLee Jul 15, 2025
9f51aca
use the logic of determining vector type for `mode` attribute
ImpleLee Jul 19, 2025
dcde7fd
clang-format
ImpleLee Jul 19, 2025
53e63bc
add proper comments for literal argument
ImpleLee Jul 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions clang/include/clang/AST/ASTContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,15 @@ class ASTContext : public RefCountedBase<ASTContext> {
QualType getIntTypeForBitwidth(unsigned DestWidth,
unsigned Signed) const;

/// getGCCCompatibleIntTypeForBitwidth -
/// sets integer QualTy according to specified details:
/// bitwidth, signed/unsigned.
/// this function is compatible with GCC's preference:
/// int > signed char > short > long > long long > int128_t
/// Returns empty type if there is no appropriate target types.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Returns empty type if there is no appropriate target types.
/// Returns null type if there is no appropriate target types.

Copy link
Author

Choose a reason for hiding this comment

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

The term empty type is also used in documentation of getIntTypeForBitwidth and getRealTypeForBitwidth series. I guess keeping them unified is probably better?

llvm-project/clang/include/clang/AST/ASTContext.h

Lines 877 to 888 in abce4e9

/// getIntTypeForBitwidth -
/// sets integer QualTy according to specified details:
/// bitwidth, signed/unsigned.
/// Returns empty type if there is no appropriate target types.
QualType getIntTypeForBitwidth(unsigned DestWidth,
unsigned Signed) const;
/// getRealTypeForBitwidth -
/// sets floating point QualTy according to specified bitwidth.
/// Returns empty type if there is no appropriate target types.
QualType getRealTypeForBitwidth(unsigned DestWidth,
FloatModeKind ExplicitType) const;

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah okay, that's pre-existing inconsistency (because in QualType itself we call this state 'Null'), so leaving it as 'empty' here is fine.

QualType getGCCCompatibleIntTypeForBitwidth(unsigned DestWidth,
unsigned Signed) const;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
unsigned Signed) const;
bool Signed) const;

Copy link
Author

Choose a reason for hiding this comment

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

The parameters are intentionally the same as the parameters of getIntTypeForBitwidth.

llvm-project/clang/include/clang/AST/ASTContext.h

Lines 877 to 882 in abce4e9

/// getIntTypeForBitwidth -
/// sets integer QualTy according to specified details:
/// bitwidth, signed/unsigned.
/// Returns empty type if there is no appropriate target types.
QualType getIntTypeForBitwidth(unsigned DestWidth,
unsigned Signed) const;

Is this a tradition in LLVM? Or I should just use bool here?


/// getRealTypeForBitwidth -
/// sets floating point QualTy according to specified bitwidth.
/// Returns empty type if there is no appropriate target types.
Expand Down
24 changes: 24 additions & 0 deletions clang/lib/AST/ASTContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13206,6 +13206,30 @@ QualType ASTContext::getIntTypeForBitwidth(unsigned DestWidth,
return QualTy;
}

/// getGCCCompatibleIntTypeForBitwidth -
/// sets integer QualTy according to specified details:
/// bitwidth, signed/unsigned.
/// this function is compatible with GCC's preference:
/// int > signed char > short > long > long long > int128_t
/// Returns empty type if there is no appropriate target types.
QualType ASTContext::getGCCCompatibleIntTypeForBitwidth(unsigned DestWidth,
unsigned Signed) const {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
unsigned Signed) const {
bool Signed) const {

const TargetInfo &Target = getTargetInfo();
if (Target.getIntWidth() == DestWidth)
return Signed ? IntTy : UnsignedIntTy;
if (Target.getCharWidth() == DestWidth)
return Signed ? SignedCharTy : UnsignedCharTy;
if (Target.getShortWidth() == DestWidth)
return Signed ? ShortTy : UnsignedShortTy;
if (Target.getLongWidth() == DestWidth)
return Signed ? LongTy : UnsignedLongTy;
if (Target.getLongLongWidth() == DestWidth)
return Signed ? LongLongTy : UnsignedLongLongTy;
if (DestWidth == 128)
return Signed ? Int128Ty : UnsignedInt128Ty;
return {};
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should probably assert if we don't find one.

Copy link
Author

Choose a reason for hiding this comment

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

The behavior of returning an empty or null type is also intentionally kept the same as getIntTypeForBitwidth. Maybe keeping this behavior can make this function a better in-place replacement of getIntTypeForBitwidth?

}

/// getRealTypeForBitwidth -
/// sets floating point QualTy according to specified bitwidth.
/// Returns empty type if there is no appropriate target types.
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Sema/SemaDeclAttr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4949,8 +4949,8 @@ void Sema::AddModeAttr(Decl *D, const AttributeCommonInfo &CI,
QualType NewElemTy;

if (IntegerMode)
NewElemTy = Context.getIntTypeForBitwidth(DestWidth,
OldElemTy->isSignedIntegerType());
NewElemTy = Context.getGCCCompatibleIntTypeForBitwidth(
DestWidth, OldElemTy->isSignedIntegerType());
else
NewElemTy = Context.getRealTypeForBitwidth(DestWidth, ExplicitType);

Expand Down
22 changes: 3 additions & 19 deletions clang/lib/Sema/SemaExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12901,25 +12901,9 @@ QualType Sema::GetSignedVectorType(QualType V) {
return Context.getExtVectorType(Context.LongLongTy, VTy->getNumElements());
}

if (TypeSize == Context.getTypeSize(Context.Int128Ty))
return Context.getVectorType(Context.Int128Ty, VTy->getNumElements(),
VectorKind::Generic);
if (TypeSize == Context.getTypeSize(Context.LongLongTy))
return Context.getVectorType(Context.LongLongTy, VTy->getNumElements(),
VectorKind::Generic);
if (TypeSize == Context.getTypeSize(Context.LongTy))
return Context.getVectorType(Context.LongTy, VTy->getNumElements(),
VectorKind::Generic);
if (TypeSize == Context.getTypeSize(Context.IntTy))
return Context.getVectorType(Context.IntTy, VTy->getNumElements(),
VectorKind::Generic);
if (TypeSize == Context.getTypeSize(Context.ShortTy))
return Context.getVectorType(Context.ShortTy, VTy->getNumElements(),
VectorKind::Generic);
assert(TypeSize == Context.getTypeSize(Context.CharTy) &&
"Unhandled vector element size in vector compare");
return Context.getVectorType(Context.CharTy, VTy->getNumElements(),
VectorKind::Generic);
QualType ETy = Context.getGCCCompatibleIntTypeForBitwidth(TypeSize, /*Signed=*/1);
assert(!ETy.isNull() && "Unhandled vector element size in vector compare");
return Context.getVectorType(ETy, VTy->getNumElements(), VectorKind::Generic);
}

QualType Sema::GetSignedSizelessVectorType(QualType V) {
Expand Down
6 changes: 4 additions & 2 deletions clang/test/Sema/vector-gcc-compat.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
// Test the compatibility of clang's vector extensions with gcc's vector
// extensions for C. Notably &&, ||, ?: and ! are not available.
typedef long long v2i64 __attribute__((vector_size(16)));
// this type is specifically used as the result type of comparison of v2i64
typedef long v2i_long __attribute__((vector_size(16)));
typedef int v2i32 __attribute__((vector_size(8)));
typedef short v2i16 __attribute__((vector_size(4)));
typedef char v2i8 __attribute__((vector_size(2)));
Expand Down Expand Up @@ -81,7 +83,7 @@ void arithmeticTest(void) {

void comparisonTest(void) {
v2i64 v2i64_a = (v2i64){0, 1};
v2i64 v2i64_r;
v2i_long v2i64_r;

v2i64_r = v2i64_a == 1;
v2i64_r = v2i64_a != 1;
Expand Down Expand Up @@ -166,7 +168,7 @@ void floatTestConstantComparison(void) {

void doubleTestConstantComparison(void) {
v2f64 v2f64_a = {0.4, 0.4};
v2i64 v2i64_r;
v2i_long v2i64_r;
v2i64_r = v2f64_a > 0.4;
v2i64_r = v2f64_a >= 0.4;
v2i64_r = v2f64_a < 0.4;
Expand Down
6 changes: 4 additions & 2 deletions clang/test/Sema/vector-gcc-compat.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
// || operators work on vector types.

typedef long long v2i64 __attribute__((vector_size(16))); // expected-warning {{'long long' is incompatible with C++98}}
// this type is specifically used as the result type of comparison of v2i64
typedef long v2i_long __attribute__((vector_size(16)));
typedef int v2i32 __attribute__((vector_size(8)));
typedef short v2i16 __attribute__((vector_size(4)));
typedef char v2i8 __attribute__((vector_size(2)));
Expand Down Expand Up @@ -60,7 +62,7 @@ void arithmeticTest(void) {

void comparisonTest(void) {
v2i64 v2i64_a = (v2i64){0, 1}; // expected-warning {{compound literals are a C99-specific feature}}
v2i64 v2i64_r;
v2i_long v2i64_r;

v2i64_r = v2i64_a == 1;
v2i64_r = v2i64_a != 1;
Expand Down Expand Up @@ -141,7 +143,7 @@ void floatTestConstantComparison(void) {

void doubleTestConstantComparison(void) {
v2f64 v2f64_a = {0.4, 0.4};
v2i64 v2i64_r;
v2i_long v2i64_r;
v2i64_r = v2f64_a > 0.4;
v2i64_r = v2f64_a >= 0.4;
v2i64_r = v2f64_a < 0.4;
Expand Down