llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-codegen Author: Nemi26 <details> <summary>Changes</summary> `__builtin_bit_cast` reads its operand through the operand's address (`CK_LValueToRValueBitCast`). Sema materialized prvalue operands, but passed glvalues through unchanged, including ones that have no address of their own: vector elements, ext_vector swizzles, matrix elements, bit-fields and global register variables. CodeGen then took the address of the containing vector, matrix or bit-field storage unit and read from its start, so for example `__builtin_bit_cast(unsigned, v.z)` returned the bits of lane 0, while constant evaluation of the same expression gave the right answer. This patch applies the lvalue-to-rvalue conversion to such operands in `CheckBuiltinBitCast` and materializes the result, as is already done for prvalues (following @<!-- -->efriedma-quic's suggestion in #<!-- -->95673). CodeGen now asserts that the operand lvalue is simple. The constant evaluator's diagnostic for an uninitialized vector element read through a bit_cast changes from "subexpression not valid" to "read of uninitialized object", which updates `constexpr-vectors-access-elements.cpp`. Fixes #<!-- -->95673 Fixes #<!-- -->219531 Fixes #<!-- -->226388 This patch was written with the assistance of an AI tool (Claude Code, Claude Opus); I reviewed it. A note on #<!-- -->219531: there is an earlier PR for it, #<!-- -->219646, but it seems to have stalled, as its author hasn't responded to the review comments for a while. I'm also aware that #<!-- -->219531 is labelled good first issue, and that the AI policy asks that such issues not be fixed with AI tools. If you'd like to keep that rule strictly for this issue, please feel free to close this PR. As the person who filed these issues, though, I'd be grateful if it could still be considered, since it also covers the other cases from #<!-- -->95673 and #<!-- -->226388. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- Full diff: https://github.com/llvm/llvm-project/pull/231830.diff 6 Files Affected: - (modified) clang/docs/ReleaseNotes.md (+4) - (modified) clang/lib/CodeGen/CGExprScalar.cpp (+1) - (modified) clang/lib/Sema/SemaCast.cpp (+10) - (added) clang/test/CodeGenCXX/builtin-bit-cast-nonsimple-lvalue.cpp (+92) - (modified) clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp (+14) - (modified) clang/test/SemaCXX/constexpr-vectors-access-elements.cpp (+9-4) ``````````diff diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 0103c29537920..e3594e4993e8e 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -756,6 +756,10 @@ features cannot lower the translation-unit ABI level; - Fixed an assertion failure when `__builtin_object_size` or `__builtin_dynamic_object_size` is called with an out-of-range type argument during constant evaluation. (#GH225041) +- Fixed wrong code generation for `__builtin_bit_cast` of a vector element, + matrix element, bit-field or global register variable, which read from the + start of the containing object instead of the element or field. + (#GH95673), (#GH219531), (#GH226388) #### Bug Fixes to Attribute Support diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index db2b7a4ad1fce..5dccebcf842a8 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -2655,6 +2655,7 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) { case CK_LValueToRValueBitCast: { LValue SourceLVal = CGF.EmitLValue(E); + assert(SourceLVal.isSimple() && "bit_cast operand has no address"); Address Addr = SourceLVal.getAddress().withElementType(CGF.ConvertTypeForMem(DestTy)); LValue DestLV = CGF.MakeAddrLValue(Addr, DestTy); diff --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp index c797bdb11e0cb..6abcd1f23288b 100644 --- a/clang/lib/Sema/SemaCast.cpp +++ b/clang/lib/Sema/SemaCast.cpp @@ -3404,6 +3404,16 @@ void CastOperation::CheckBuiltinBitCast() { return; } + // An operand without an address of its own is loaded, then materialized. + if (SrcExpr.get()->isGLValue() && + (SrcExpr.get()->getObjectKind() != OK_Ordinary || + SrcExpr.get()->refersToGlobalRegisterVar())) { + SrcExpr = Self.DefaultLvalueConversion(SrcExpr.get()); + if (SrcExpr.isInvalid()) + return; + SrcType = SrcExpr.get()->getType(); + } + if (SrcExpr.get()->isPRValue()) SrcExpr = Self.CreateMaterializeTemporaryExpr(SrcType, SrcExpr.get(), /*IsLValueReference=*/false); diff --git a/clang/test/CodeGenCXX/builtin-bit-cast-nonsimple-lvalue.cpp b/clang/test/CodeGenCXX/builtin-bit-cast-nonsimple-lvalue.cpp new file mode 100644 index 0000000000000..8818c714d73b8 --- /dev/null +++ b/clang/test/CodeGenCXX/builtin-bit-cast-nonsimple-lvalue.cpp @@ -0,0 +1,92 @@ +// RUN: %clang_cc1 -std=c++20 -fenable-matrix -emit-llvm -disable-llvm-passes -triple x86_64-unknown-linux-gnu %s -o - | FileCheck %s + +// An operand without an address of its own is loaded through the lvalue and +// materialized, rather than read from the start of the containing object. + +typedef float v4f __attribute__((ext_vector_type(4))); +typedef float gv4f __attribute__((vector_size(16))); +typedef float m2x2f __attribute__((matrix_type(2, 2))); + +// CHECK-LABEL: define{{.*}} i32 @_Z18ext_vector_elementDv4_f( +// CHECK: [[TMP:%.*]] = alloca float +// CHECK: [[VEC:%.*]] = load <4 x float>, ptr %v.addr +// CHECK-NEXT: [[ELT:%.*]] = extractelement <4 x float> [[VEC]], i64 2 +// CHECK-NEXT: store float [[ELT]], ptr [[TMP]] +// CHECK-NEXT: [[RES:%.*]] = load i32, ptr [[TMP]] +// CHECK-NEXT: ret i32 [[RES]] +unsigned ext_vector_element(v4f v) { + return __builtin_bit_cast(unsigned, v.z); +} + +// CHECK-LABEL: define{{.*}} i32 @_Z20ext_vector_subscriptDv4_f( +// CHECK: [[TMP:%.*]] = alloca float +// CHECK: [[VEC:%.*]] = load <4 x float>, ptr %v.addr +// CHECK-NEXT: [[ELT:%.*]] = extractelement <4 x float> [[VEC]], i32 1 +// CHECK-NEXT: store float [[ELT]], ptr [[TMP]] +// CHECK-NEXT: [[RES:%.*]] = load i32, ptr [[TMP]] +// CHECK-NEXT: ret i32 [[RES]] +unsigned ext_vector_subscript(v4f v) { + return __builtin_bit_cast(unsigned, v[1]); +} + +// CHECK-LABEL: define{{.*}} i64 @_Z18ext_vector_swizzleDv4_f( +// CHECK: [[TMP:%.*]] = alloca <2 x float> +// CHECK: [[VEC:%.*]] = load <4 x float>, ptr %v.addr +// CHECK-NEXT: [[ELTS:%.*]] = shufflevector <4 x float> [[VEC]], <4 x float> poison, <2 x i32> <i32 2, i32 3> +// CHECK-NEXT: store <2 x float> [[ELTS]], ptr [[TMP]] +// CHECK-NEXT: [[RES:%.*]] = load i64, ptr [[TMP]] +// CHECK-NEXT: ret i64 [[RES]] +unsigned long long ext_vector_swizzle(v4f v) { + return __builtin_bit_cast(unsigned long long, v.zw); +} + +// CHECK-LABEL: define{{.*}} i32 @_Z20gnu_vector_subscriptDv4_fi( +// CHECK: [[TMP:%.*]] = alloca float +// CHECK: [[VEC:%.*]] = load <4 x float>, ptr %v.addr +// CHECK-NEXT: [[IDX:%.*]] = load i32, ptr %i.addr +// CHECK-NEXT: [[ELT:%.*]] = extractelement <4 x float> [[VEC]], i32 [[IDX]] +// CHECK-NEXT: store float [[ELT]], ptr [[TMP]] +// CHECK-NEXT: [[RES:%.*]] = load i32, ptr [[TMP]] +// CHECK-NEXT: ret i32 [[RES]] +unsigned gnu_vector_subscript(gv4f v, int i) { + return __builtin_bit_cast(unsigned, v[i]); +} + +// CHECK-LABEL: define{{.*}} i32 @_Z14matrix_elementu11matrix_typeILm2ELm2EfE( +// CHECK: [[TMP:%.*]] = alloca float +// CHECK: [[MAT:%.*]] = load <4 x float>, ptr %m.addr +// CHECK-NEXT: [[ELT:%.*]] = extractelement <4 x float> [[MAT]], i64 3 +// CHECK-NEXT: store float [[ELT]], ptr [[TMP]] +// CHECK-NEXT: [[RES:%.*]] = load i32, ptr [[TMP]] +// CHECK-NEXT: ret i32 [[RES]] +unsigned matrix_element(m2x2f m) { + return __builtin_bit_cast(unsigned, m[1][1]); +} + +struct S { + unsigned a : 4, b : 12; +}; + +// CHECK-LABEL: define{{.*}} i32 @_Z9bit_field1S( +// CHECK: [[TMP:%.*]] = alloca i32 +// CHECK: [[LOAD:%.*]] = load i16, ptr %s +// CHECK-NEXT: [[SHR:%.*]] = lshr i16 [[LOAD]], 4 +// CHECK-NEXT: [[EXT:%.*]] = zext i16 [[SHR]] to i32 +// CHECK-NEXT: store i32 [[EXT]], ptr [[TMP]] +// CHECK-NEXT: [[RES:%.*]] = load i32, ptr [[TMP]] +// CHECK-NEXT: ret i32 [[RES]] +unsigned bit_field(S s) { + return __builtin_bit_cast(unsigned, s.b); +} + +register unsigned long stack_pointer asm("rsp"); + +// CHECK-LABEL: define{{.*}} i64 @_Z15global_registerv( +// CHECK: [[TMP:%.*]] = alloca i64 +// CHECK: [[REG:%.*]] = call i64 @llvm.read_register.i64( +// CHECK-NEXT: store i64 [[REG]], ptr [[TMP]] +// CHECK-NEXT: [[RES:%.*]] = load i64, ptr [[TMP]] +// CHECK-NEXT: ret i64 [[RES]] +long global_register() { + return __builtin_bit_cast(long, stack_pointer); +} diff --git a/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp b/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp index ea5333df0b469..db53076891b47 100644 --- a/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp +++ b/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp @@ -491,6 +491,20 @@ static_assert(bit_cast<unsigned long long>(test_vector) == (LITTLE_END static_assert(round_trip<uint2>(0xCAFEBABE0C05FEFEULL), ""); static_assert(round_trip<byte8>(0xCAFEBABE0C05FEFEULL), ""); +typedef unsigned uint2e __attribute__((ext_vector_type(2))); +constexpr uint2e test_ext_vector = { 0x0C05FEFE, 0xCAFEBABE }; + +// Elements are read by value, not from the start of the vector. +static_assert(__builtin_bit_cast(unsigned, test_vector[1]) == 0xCAFEBABE, ""); +static_assert(__builtin_bit_cast(unsigned, test_ext_vector[1]) == 0xCAFEBABE, ""); +static_assert(__builtin_bit_cast(unsigned, test_ext_vector.y) == 0xCAFEBABE, ""); + +struct BitField { + unsigned a : 4, b : 12; +}; +constexpr BitField test_bit_field = { 0xF, 0x123 }; +static_assert(__builtin_bit_cast(unsigned, test_bit_field.b) == 0x123, ""); + typedef bool bool8 __attribute__((ext_vector_type(8))); typedef bool bool9 __attribute__((ext_vector_type(9))); typedef bool bool16 __attribute__((ext_vector_type(16))); diff --git a/clang/test/SemaCXX/constexpr-vectors-access-elements.cpp b/clang/test/SemaCXX/constexpr-vectors-access-elements.cpp index 481b80ceb9c1e..c6eb66f6b2995 100644 --- a/clang/test/SemaCXX/constexpr-vectors-access-elements.cpp +++ b/clang/test/SemaCXX/constexpr-vectors-access-elements.cpp @@ -65,17 +65,22 @@ static_assert(t2[15] == -2); using I = int __attribute__((vector_size(16))); -// expected-error@+1 {{constexpr function never produces a constant expression}} constexpr unsigned test2() { - // cpp17-warning@+1 {{uninitialized variable in a constexpr function is a C++20 extension}} + // cpp17-warning@+2 {{uninitialized variable in a constexpr function is a C++20 extension}} + // expected-note@+1 {{declared here}} I v; - // expected-note@+2 {{subexpression not valid in a constant expression}} - // expected-note@+1 {{subexpression not valid in a constant expression}} + // expected-note@+1 {{read of uninitialized object is not allowed in a constant expression}} return __builtin_bit_cast(unsigned, v[0]); } // expected-error@+2 {{static assertion expression is not an integral constant expression}} // expected-note@+1 {{in call to 'test2()'}} static_assert(test2(), ""); + +constexpr unsigned test3() { + I v = {1, 2, 3, 4}; + return __builtin_bit_cast(unsigned, v[1]); +} +static_assert(test3() == 2, ""); } `````````` </details> https://github.com/llvm/llvm-project/pull/231830 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
