[clang][NFC] Refactor CodeGen's hasBooleanRepresentation

[andykaylor]

The ClangIR upstreaming project needs the same logic for hasBooleanRepresentation() that is currently implemented in the standard clang codegen. In order to share this code, this change moves the implementation of this function into the AST Type class.

No functional change is intended by this change. The ClangIR use of this function will be added separately in a later change.

[llvmbot]

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Andy Kaylor (andykaylor)

Changes

The ClangIR upstreaming project needs the same logic for hasBooleanRepresentation() that is currently implemented in the standard clang codegen. In order to share this code, this change moves the implementation of this function into the AST Type class.

No functional change is intended by this change. The ClangIR use of this function will be added separately in a later change.

---

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

3 Files Affected:

• (modified) clang/include/clang/AST/Type.h (+4)
• (modified) clang/lib/AST/Type.cpp (+13)
• (modified) clang/lib/CodeGen/CGExpr.cpp (+5-18)

diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index a809102c069a8..c38250a13a6e7 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -2769,6 +2769,10 @@ 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.
+  bool hasBooleanRepresentation() const;
+
   // Type Checking Functions: Check to see if this type is structurally the
   // specified type, ignoring typedefs and qualifiers, and return a pointer to
   // the best type we can.
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 08798219c0b83..42fec3a13cf2f 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -2334,6 +2334,19 @@ bool Type::isArithmeticType() const {
   return isa<ComplexType>(CanonicalType) || isBitIntType();
 }
 
+bool Type::hasBooleanRepresentation() const {
+  if (isBooleanType())
+    return true;
+
+  if (const EnumType *ET = getAs<EnumType>())
+    return ET->getDecl()->getIntegerType()->isBooleanType();
+
+  if (const AtomicType *AT = getAs<AtomicType>())
+    return AT->getValueType()->hasBooleanRepresentation();
+
+  return false;
+}
+
 Type::ScalarTypeKind Type::getScalarTypeKind() const {
   assert(isScalarType());
 
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 3d3a111f0514a..1bde01490a428 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -1893,19 +1893,6 @@ llvm::Value *CodeGenFunction::EmitLoadOfScalar(LValue lvalue,
                           lvalue.getTBAAInfo(), lvalue.isNontemporal());
 }
 
-static bool hasBooleanRepresentation(QualType Ty) {
-  if (Ty->isBooleanType())
-    return true;
-
-  if (const EnumType *ET = Ty->getAs<EnumType>())
-    return ET->getDecl()->getIntegerType()->isBooleanType();
-
-  if (const AtomicType *AT = Ty->getAs<AtomicType>())
-    return hasBooleanRepresentation(AT->getValueType());
-
-  return false;
-}
-
 static bool getRangeForType(CodeGenFunction &CGF, QualType Ty,
                             llvm::APInt &Min, llvm::APInt &End,
                             bool StrictEnums, bool IsBool) {
@@ -1928,7 +1915,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,
-                       hasBooleanRepresentation(Ty)))
+                       Ty->hasBooleanRepresentation()))
     return nullptr;
 
   llvm::MDBuilder MDHelper(getLLVMContext());
@@ -1942,7 +1929,7 @@ bool CodeGenFunction::EmitScalarRangeCheck(llvm::Value *Value, QualType Ty,
   if (!HasBoolCheck && !HasEnumCheck)
     return false;
 
-  bool IsBool = hasBooleanRepresentation(Ty) ||
+  bool IsBool = Ty->hasBooleanRepresentation() ||
                 NSAPI(CGM.getContext()).isObjCBOOLType(Ty);
   bool NeedsBoolCheck = HasBoolCheck && IsBool;
   bool NeedsEnumCheck = HasEnumCheck && Ty->getAs<EnumType>();
@@ -2070,7 +2057,7 @@ 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 (hasBooleanRepresentation(Ty) || Ty->isBitIntType()) {
+  if (Ty->hasBooleanRepresentation() || Ty->isBitIntType()) {
     llvm::Type *StoreTy = convertTypeForLoadStore(Ty, Value->getType());
     bool Signed = Ty->isSignedIntegerOrEnumerationType();
     return Builder.CreateIntCast(Value, StoreTy, Signed, "storedv");
@@ -2111,7 +2098,7 @@ llvm::Value *CodeGenFunction::EmitFromMemory(llvm::Value *Value, QualType Ty) {
   }
 
   llvm::Type *ResTy = ConvertType(Ty);
-  if (hasBooleanRepresentation(Ty) || Ty->isBitIntType() ||
+  if (Ty->hasBooleanRepresentation() || Ty->isBitIntType() ||
       Ty->isExtVectorBoolType())
     return Builder.CreateTrunc(Value, ResTy, "loadedv");
 
@@ -2598,7 +2585,7 @@ void CodeGenFunction::EmitStoreThroughBitfieldLValue(RValue Src, LValue Dst,
         Builder.CreateLoad(Ptr, Dst.isVolatileQualified(), "bf.load");
 
     // Mask the source value as needed.
-    if (!hasBooleanRepresentation(Dst.getType()))
+    if (!Dst.getType()->hasBooleanRepresentation())
       SrcVal = Builder.CreateAnd(
           SrcVal, llvm::APInt::getLowBitsSet(StorageSize, Info.Size),
           "bf.value");

[michele-scandale]

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(<type with boolean representation>) to have hasBooleanRepresentation returning false.
Any thoughts?

[andykaylor]

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(<type with boolean representation>) to have hasBooleanRepresentation returning false. Any thoughts?

Yes, I also noticed that when I was moving this. I wasn't sure which way it should go, so I opted to keep the code the same as it was in its original location. There is already a precedent (hasPointerRepresentation) for not including vectors. Also, vectors of Booleans have different considerations that scalar Booleans. For instance, they are stored differently in memory.

Maybe the function should have a different name to avoid confusion?

[michele-scandale]

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.

[michele-scandale]

See #136038