[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

[Diogo N. Sampaio via Phabricator via cfe-commits]

dnsampaio created this revision.
dnsampaio added reviewers: lebedev.ri, ostannard.
Herald added subscribers: cfe-commits, jfb, kristof.beyls.
Herald added a project: clang.

Bug 43264
This is a first draft to understand what has to be
done to fix volatale bitfield access, as to conform
to the AACPS


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67399

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGen/aapcs-bitfield.c


Index: clang/test/CodeGen/aapcs-bitfield.c
===================================================================
--- clang/test/CodeGen/aapcs-bitfield.c
+++ clang/test/CodeGen/aapcs-bitfield.c
@@ -531,12 +531,14 @@
 // LE-LABEL: @store_st9(
 // LE-NEXT:  entry:
 // LE-NEXT:    [[TMP0:%.*]] = getelementptr inbounds [[STRUCT_ST9:%.*]], %struct.st9* [[M:%.*]], i32 0, i32 0
+// LE-NEXT:    [[BF_LOAD:%.*]] = load volatile i8, i8* [[TMP0]], align 4
 // LE-NEXT:    store volatile i8 1, i8* [[TMP0]], align 4
 // LE-NEXT:    ret void
 //
 // BE-LABEL: @store_st9(
 // BE-NEXT:  entry:
 // BE-NEXT:    [[TMP0:%.*]] = getelementptr inbounds [[STRUCT_ST9:%.*]], %struct.st9* [[M:%.*]], i32 0, i32 0
+// BE-NEXT:    [[BF_LOAD:%.*]] = load volatile i8, i8* [[TMP0]], align 4
 // BE-NEXT:    store volatile i8 1, i8* [[TMP0]], align 4
 // BE-NEXT:    ret void
 //
@@ -549,6 +551,7 @@
 // LE-NEXT:    [[TMP0:%.*]] = getelementptr inbounds [[STRUCT_ST9:%.*]], %struct.st9* [[M:%.*]], i32 0, i32 0
 // LE-NEXT:    [[BF_LOAD:%.*]] = load volatile i8, i8* [[TMP0]], align 4
 // LE-NEXT:    [[INC:%.*]] = add i8 [[BF_LOAD]], 1
+// LE-NEXT:    [[BF_LOAD1:%.*]] = load volatile i8, i8* [[TMP0]], align 4
 // LE-NEXT:    store volatile i8 [[INC]], i8* [[TMP0]], align 4
 // LE-NEXT:    ret void
 //
@@ -557,6 +560,7 @@
 // BE-NEXT:    [[TMP0:%.*]] = getelementptr inbounds [[STRUCT_ST9:%.*]], %struct.st9* [[M:%.*]], i32 0, i32 0
 // BE-NEXT:    [[BF_LOAD:%.*]] = load volatile i8, i8* [[TMP0]], align 4
 // BE-NEXT:    [[INC:%.*]] = add i8 [[BF_LOAD]], 1
+// BE-NEXT:    [[BF_LOAD1:%.*]] = load volatile i8, i8* [[TMP0]], align 4
 // BE-NEXT:    store volatile i8 [[INC]], i8* [[TMP0]], align 4
 // BE-NEXT:    ret void
 //
@@ -667,12 +671,14 @@
 // LE-LABEL: @store_st11(
 // LE-NEXT:  entry:
 // LE-NEXT:    [[F:%.*]] = getelementptr inbounds [[STRUCT_ST11:%.*]], %struct.st11* [[M:%.*]], i32 0, i32 1
+// LE-NEXT:    [[BF_LOAD:%.*]] = load volatile i16, i16* [[F]], align 1
 // LE-NEXT:    store volatile i16 1, i16* [[F]], align 1
 // LE-NEXT:    ret void
 //
 // BE-LABEL: @store_st11(
 // BE-NEXT:  entry:
 // BE-NEXT:    [[F:%.*]] = getelementptr inbounds [[STRUCT_ST11:%.*]], %struct.st11* [[M:%.*]], i32 0, i32 1
+// BE-NEXT:    [[BF_LOAD:%.*]] = load volatile i16, i16* [[F]], align 1
 // BE-NEXT:    store volatile i16 1, i16* [[F]], align 1
 // BE-NEXT:    ret void
 //
@@ -685,6 +691,7 @@
 // LE-NEXT:    [[F:%.*]] = getelementptr inbounds [[STRUCT_ST11:%.*]], %struct.st11* [[M:%.*]], i32 0, i32 1
 // LE-NEXT:    [[BF_LOAD:%.*]] = load volatile i16, i16* [[F]], align 1
 // LE-NEXT:    [[INC:%.*]] = add i16 [[BF_LOAD]], 1
+// LE-NEXT:    [[BF_LOAD1:%.*]] = load volatile i16, i16* [[F]], align 1
 // LE-NEXT:    store volatile i16 [[INC]], i16* [[F]], align 1
 // LE-NEXT:    ret void
 //
@@ -693,6 +700,7 @@
 // BE-NEXT:    [[F:%.*]] = getelementptr inbounds [[STRUCT_ST11:%.*]], %struct.st11* [[M:%.*]], i32 0, i32 1
 // BE-NEXT:    [[BF_LOAD:%.*]] = load volatile i16, i16* [[F]], align 1
 // BE-NEXT:    [[INC:%.*]] = add i16 [[BF_LOAD]], 1
+// BE-NEXT:    [[BF_LOAD1:%.*]] = load volatile i16, i16* [[F]], align 1
 // BE-NEXT:    store volatile i16 [[INC]], i16* [[F]], align 1
 // BE-NEXT:    ret void
 //
Index: clang/lib/CodeGen/CGExpr.cpp
===================================================================
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -2061,6 +2061,13 @@
     SrcVal = Builder.CreateOr(Val, SrcVal, "bf.set");
   } else {
     assert(Info.Offset == 0);
+    // Acording to the AACPS:
+    // When a volatile bit-field is written, and its container does not overlap
+    // with any non-bit-field member, its container must be read exactly once and
+    // written exactly once using the access width appropriate to the type of the
+    // container. The two accesses are not atomic.
+    if ( (CGM.getTarget().getABI() == "aapcs") && Dst.isVolatileQualified() )
+      Builder.CreateLoad(Ptr, Dst.isVolatileQualified(), "bf.load");
   }
 
   // Write the new value back out.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67399.219542.patch
Type: text/x-patch
Size: 4022 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190910/490c6c79/attachment.bin>