Author: Alex Bradbury Date: 2020-06-12T10:33:47+01:00 New Revision: 3dcfd482cb17fad2d641c976b822d1fe36dc1359
URL: https://github.com/llvm/llvm-project/commit/3dcfd482cb17fad2d641c976b822d1fe36dc1359 DIFF: https://github.com/llvm/llvm-project/commit/3dcfd482cb17fad2d641c976b822d1fe36dc1359.diff LOG: [CodeGen] Increase applicability of ffine-grained-bitfield-accesses for targets with limited native integer widths As pointed out in PR45708, -ffine-grained-bitfield-accesses doesn't trigger in all cases you think it might for RISC-V. The logic in CGRecordLowering::accumulateBitFields checks OffsetInRecord is a legal integer according to the datalayout. RISC targets will typically only have the native width as a legal integer type so this check will fail for OffsetInRecord of 8 or 16 when you would expect the transformation is still worthwhile. This patch changes the logic to check for an OffsetInRecord of a at least 1 byte, that fits in a legal integer, and is a power of 2. We would prefer to query whether native load/store operations are available, but I don't believe that is possible. Differential Revision: https://reviews.llvm.org/D79155 Added: Modified: clang/lib/CodeGen/CGRecordLayoutBuilder.cpp clang/test/CodeGenCXX/finegrain-bitfield-type.cpp Removed: ################################################################################ diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp index c9a1b1d4fd14..4e5d1d3f16f6 100644 --- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp +++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp @@ -406,15 +406,17 @@ CGRecordLowering::accumulateBitFields(RecordDecl::field_iterator Field, return; } - // Check if OffsetInRecord is better as a single field run. When OffsetInRecord - // has legal integer width, and its bitfield offset is naturally aligned, it - // is better to make the bitfield a separate storage component so as it can be - // accessed directly with lower cost. + // Check if OffsetInRecord (the size in bits of the current run) is better + // as a single field run. When OffsetInRecord has legal integer width, and + // its bitfield offset is naturally aligned, it is better to make the + // bitfield a separate storage component so as it can be accessed directly + // with lower cost. auto IsBetterAsSingleFieldRun = [&](uint64_t OffsetInRecord, uint64_t StartBitOffset) { if (!Types.getCodeGenOpts().FineGrainedBitfieldAccesses) return false; - if (!DataLayout.isLegalInteger(OffsetInRecord)) + if (OffsetInRecord < 8 || !llvm::isPowerOf2_64(OffsetInRecord) || + !DataLayout.fitsInLegalInteger(OffsetInRecord)) return false; // Make sure StartBitOffset is natually aligned if it is treated as an // IType integer. diff --git a/clang/test/CodeGenCXX/finegrain-bitfield-type.cpp b/clang/test/CodeGenCXX/finegrain-bitfield-type.cpp index ff02d46de5e4..2106a6d3e0a9 100644 --- a/clang/test/CodeGenCXX/finegrain-bitfield-type.cpp +++ b/clang/test/CodeGenCXX/finegrain-bitfield-type.cpp @@ -1,5 +1,12 @@ // RUN: %clang_cc1 -triple x86_64-linux-gnu -ffine-grained-bitfield-accesses \ // RUN: -emit-llvm -o - %s | FileCheck %s +// RUN: %clang_cc1 -triple riscv64-linux-gnu -ffine-grained-bitfield-accesses \ +// RUN: -emit-llvm -o - %s | FileCheck %s + +// Note: This test checks the X86-64 and RISC-V targets in order to explore +// behaviour when i8/i16 are native integer widths (X86-64) and when they're +// not (RISC-V). + struct S4 { unsigned long f1:28; unsigned long f2:4; @@ -19,4 +26,4 @@ struct S5 a5; // CHECK: %struct.S4 = type { i32, i16 } // CHECK-NOT: %struct.S4 = type { i48 } // CHECK: %struct.S5 = type { i32, i32, i16, [6 x i8] } -// CHECK-NOT: %struct.S5 = type { i80 } \ No newline at end of file +// CHECK-NOT: %struct.S5 = type { i80 } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
