llvmbot wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-codegen

Author: Kamran Yousafzai (KamranYousafzai)

<details>
<summary>Changes</summary>

The code generated for calls with FPCC eligible structs as arguments doesn't 
consider the alignment with a bitfield, which results in a store crossing the 
boundary of the memory allocated using alloca, e.g.
For the code:
```
struct __attribute__((packed, aligned(1))) S {
   const float  f0;
   unsigned f1 : 1;
};
unsigned  func(struct S  arg)
{
    return arg.f1;
} 
```
The generated IR is:
```
 define dso_local signext i32 @<!-- -->func(
 float [[TMP0:%.*]], i32 [[TMP1:%.*]]) #[[ATTR0:[0-9]+]] {
  [[ENTRY:.*:]]
    [[ARG:%.*]] = alloca [[STRUCT_S:%.*]], align 1
    [[TMP2:%.*]] = getelementptr inbounds nuw { float, i32 }, ptr [[ARG]], i32 
0, i32 0
    store float [[TMP0]], ptr [[TMP2]], align 1
    [[TMP3:%.*]] = getelementptr inbounds nuw { float, i32 }, ptr [[ARG]], i32 
0, i32 1
    store i32 [[TMP1]], ptr [[TMP3]], align 1
    [[F1:%.*]] = getelementptr inbounds nuw [[STRUCT_S]], ptr [[ARG]], i32 0, 
i32 1
    [[BF_LOAD:%.*]] = load i8, ptr [[F1]], align 1
    [[BF_CLEAR:%.*]] = and i8 [[BF_LOAD]], 1
    [[BF_CAST:%.*]] = zext i8 [[BF_CLEAR]] to i32
    ret i32 [[BF_CAST]]
```
Where, `store i32 [[TMP1]], ptr [[TMP3]], align 1` can be seen to crossing the 
boundary of the allocated memory. If, the IR is seen after optimizations, the 
IR left is:
```
 define dso_local noundef signext i32 @<!-- -->func(
 float [[TMP0:%.*]], i32 [[TMP1:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
  [[ENTRY:.*:]]
    ret i32 0
```
The patch trims the second member of the struct after taking into consideration 
the alignment and bitwidth to decide the appropriate integer type and the test 
shows the results of this patch.

Note that the bug is seen only when `f` extension is enabled.

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


2 Files Affected:

- (modified) clang/lib/CodeGen/Targets/RISCV.cpp (+11) 
- (added) clang/test/CodeGen/RISCV/riscv-fpcc-struct.c (+21) 


``````````diff
diff --git a/clang/lib/CodeGen/Targets/RISCV.cpp 
b/clang/lib/CodeGen/Targets/RISCV.cpp
index fd72fe673b9b14..142371ffe27e54 100644
--- a/clang/lib/CodeGen/Targets/RISCV.cpp
+++ b/clang/lib/CodeGen/Targets/RISCV.cpp
@@ -224,6 +224,8 @@ bool RISCVABIInfo::detectFPCCEligibleStructHelper(QualType 
Ty, CharUnits CurOff,
     if (isEmptyRecord(getContext(), Ty, true, true))
       return true;
     const RecordDecl *RD = RTy->getDecl();
+    const Type *RT = RD->getTypeForDecl();
+    unsigned Alignment = getContext().getTypeAlign(RT);
     // Unions aren't eligible unless they're empty (which is caught above).
     if (RD->isUnion())
       return false;
@@ -251,6 +253,15 @@ bool RISCVABIInfo::detectFPCCEligibleStructHelper(QualType 
Ty, CharUnits CurOff,
         // bitwidth is XLen or less.
         if (getContext().getTypeSize(QTy) > XLen && BitWidth <= XLen)
           QTy = getContext().getIntTypeForBitwidth(XLen, false);
+        // Trim type to alignment/bitwidth if that is possible
+        else if (getContext().getTypeSize(QTy) > Alignment &&
+                 getContext().getTypeSize(QTy) > BitWidth) {
+          bool isSigned =
+              FD->getType().getTypePtr()->hasSignedIntegerRepresentation();
+          unsigned bits =
+              std::max(Alignment, (unsigned)llvm::PowerOf2Ceil(BitWidth));
+          QTy = getContext().getIntTypeForBitwidth(bits, isSigned);
+        }
         if (BitWidth == 0) {
           ZeroWidthBitFieldCount++;
           continue;
diff --git a/clang/test/CodeGen/RISCV/riscv-fpcc-struct.c 
b/clang/test/CodeGen/RISCV/riscv-fpcc-struct.c
new file mode 100644
index 00000000000000..5d813aa05e60c6
--- /dev/null
+++ b/clang/test/CodeGen/RISCV/riscv-fpcc-struct.c
@@ -0,0 +1,21 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py 
UTC_ARGS: --version 5
+// RUN: %clang_cc1 -triple riscv64 -target-feature +f -emit-llvm -O3 %s -o - \
+// RUN:   | FileCheck %s
+
+
+struct __attribute__((packed, aligned(1))) S {
+   const float  f0;
+   unsigned f1 : 1;
+};
+
+// CHECK-LABEL: define dso_local signext range(i32 0, 2) i32 @func(
+// CHECK-SAME: float [[TMP0:%.*]], i8 [[TMP1:%.*]]) local_unnamed_addr 
#[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[BF_CLEAR:%.*]] = and i8 [[TMP1]], 1
+// CHECK-NEXT:    [[BF_CAST:%.*]] = zext nneg i8 [[BF_CLEAR]] to i32
+// CHECK-NEXT:    ret i32 [[BF_CAST]]
+//
+unsigned  func(struct S  arg)
+{
+    return arg.f1;
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/110690
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to