[clang] 3dcfd48 - [CodeGen] Increase applicability of ffine-grained-bitfield-accesses for targets with limited native integer widths

[Alex Bradbury via cfe-commits]

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 }