[PATCH] D79155: [CodeGen] Increase applicability of ffine-grained-bitfield-accesses for targets with limited native integer widths

[Alex Bradbury via Phabricator via cfe-commits]

asb created this revision.
asb added reviewers: wmi, hfinkel, efriedma.
Herald added subscribers: cfe-commits, luismarques, sameer.abuasal, s.egerton, lenary, PkmX, simoncook.
Herald added a project: clang.
asb added a subscriber: apazos.

As pointed out in PR45708 <https://bugs.llvm.org/show_bug.cgi?id=45708>, `-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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79155

Files:
  clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
  clang/test/CodeGenCXX/finegrain-bitfield-type.cpp


Index: clang/test/CodeGenCXX/finegrain-bitfield-type.cpp
===================================================================
--- clang/test/CodeGenCXX/finegrain-bitfield-type.cpp
+++ 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 @@
 // 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 }
Index: clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
===================================================================
--- clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -414,7 +414,8 @@
                                       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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79155.261137.patch
Type: text/x-patch
Size: 1691 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200430/5aa5415c/attachment.bin>