Re: r341475 - Fix arm_neon.h and arm_fp16.h generation for compiling with std=c89
Sorry, but my patch is not reverted, and as for now it only reads: Failing Tests (2): LLVM :: CodeGen/AMDGPU/mubuf-legalize-operands.ll LLVM :: CodeGen/AMDGPU/mubuf-legalize-operands.mir So I'm considering it was a side-effect of some other test. On 09/05/2018 10:00 PM, Galina Kistanova wrote: Hello Diogo, This commit added couple of broken tests to one of our builders: http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win . . . Failing Tests (5): Clang :: Headers/arm-fp16-header.c Clang :: Headers/arm-neon-header.c . . . Please have a look? The builder was already red and did not send notifications on this. Thanks Galina On Wed, Sep 5, 2018 at 7:59 AM Diogo N. Sampaio via cfe-commits mailto:cfe-commits@lists.llvm.org>> wrote: Author: dnsampaio Date: Wed Sep 5 07:56:21 2018 New Revision: 341475 URL: http://llvm.org/viewvc/llvm-project?rev=341475&view=rev Log: Fix arm_neon.h and arm_fp16.h generation for compiling with std=c89 Summary: The inline attribute is not valid for C standard 89. Replace the argument in the generation of header files with __inline, as well adding tests for both header files. Reviewers: pbarrio, SjoerdMeijer, javed.absar, t.p.northover Subscribers: t.p.northover, kristof.beyls, chrib, cfe-commits Differential Revision: https://reviews.llvm.org/D51683 test/Headers/arm-fp16-header.c test/Headers/arm-neon-header.c utils/TableGen/NeonEmitter.cpp Added: cfe/trunk/test/Headers/arm-fp16-header.c Modified: cfe/trunk/test/Headers/arm-neon-header.c cfe/trunk/utils/TableGen/NeonEmitter.cpp Added: cfe/trunk/test/Headers/arm-fp16-header.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/arm-fp16-header.c?rev=341475&view=auto == --- cfe/trunk/test/Headers/arm-fp16-header.c (added) +++ cfe/trunk/test/Headers/arm-fp16-header.c Wed Sep 5 07:56:21 2018 @@ -0,0 +1,19 @@ +// RUN: %clang -fsyntax-only -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c99 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c11 -xc %s + +// RUN: %clang -fsyntax-only -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c99 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c11 -xc %s + +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++98 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++11 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++14 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c++17 -xc++ %s + +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++98 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++11 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++14 -xc++ %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c++17 -xc++ %s + +#include Modified: cfe/trunk/test/Headers/arm-neon-header.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/arm-neon-header.c?rev=341475&r1=341474&r2=341475&view=diff == --- cfe/trunk/test/Headers/arm-neon-header.c (original) +++ cfe/trunk/test/Headers/arm-neon-header.c Wed Sep 5 07:56:21 2018 @@ -2,4 +2,23 @@ // RUN: %clang_cc1 -triple thumbv7-apple-darwin10 -target-cpu cortex-a8 -fsyntax-only -fno-lax-vector-conversions -ffreestanding %s // RUN: %clang_cc1 -x c++ -triple thumbv7-apple-darwin10 -target-cpu cortex-a8 -fsyntax-only -Wvector-conversions -ffreestanding %s +// RUN: %clang -fsyntax-only -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c99 -xc %s +// RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-arm-none-eabi -march=armv8.2-a+fp16 -std=c11 -xc %s + +// RUN: %clang -fsyntax-only -ffreestanding --target=aarch64-armeb-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s +// RUN: %clang -fsyntax-only -Wall
[clang] 6a24339 - [ARM] Follow AACPS standard for volatile bit-fields access width
Author: Diogo Sampaio
Date: 2020-01-21T15:23:38Z
New Revision: 6a24339a45246b66bd3de88cc9c6a5b5e77c0645
URL:
https://github.com/llvm/llvm-project/commit/6a24339a45246b66bd3de88cc9c6a5b5e77c0645
DIFF:
https://github.com/llvm/llvm-project/commit/6a24339a45246b66bd3de88cc9c6a5b5e77c0645.diff
LOG: [ARM] Follow AACPS standard for volatile bit-fields access width
Summary:
This patch resumes the work of D16586.
According to the AAPCS, volatile bit-fields should
be accessed using containers of the widht of their
declarative type. In such case:
```
struct S1 {
short a : 1;
}
```
should be accessed using load and stores of the width
(sizeof(short)), where now the compiler does only load
the minimum required width (char in this case).
However, as discussed in D16586,
that could overwrite non-volatile bit-fields, which
conflicted with C and C++ object models by creating
data race conditions that are not part of the bit-field,
e.g.
```
struct S2 {
short a;
int b : 16;
}
```
Accessing `S2.b` would also access `S2.a`.
The AAPCS Release 2019Q1.1
(https://static.docs.arm.com/ihi0042/g/aapcs32.pdf)
section 8.1 Data Types, page 35, "Volatile bit-fields -
preserving number and width of container accesses" has been
updated to avoid conflict with the C++ Memory Model.
Now it reads in the note:
```
This ABI does not place any restrictions on the access widths
of bit-fields where the container overlaps with a non-bit-field member.
This is because the C/C++ memory model defines these as being separate
memory locations, which can be accessed by two threads
simultaneously. For this reason, compilers must be permitted to use a
narrower memory access width (including splitting the access
into multiple instructions) to avoid writing to a different memory location.
```
I've updated the patch D16586 to follow such behavior by verifying that we
only change volatile bit-field access when:
- it won't overlap with any other non-bit-field member
- we only access memory inside the bounds of the record
Regarding the number of memory accesses, that should be preserved, that will
be implemented by D67399.
Reviewers: rsmith, rjmccall, eli.friedman, ostannard
Subscribers: ostannard, kristof.beyls, cfe-commits, carwil, olista01
Tags: #clang
Differential Revision: https://reviews.llvm.org/D72932
Added:
Modified:
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGValue.h
clang/lib/CodeGen/CodeGenFunction.h
clang/test/CodeGen/aapcs-bitfield.c
Removed:
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 8e0604181fb1..c4029c72dd5f 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -177,6 +177,11 @@ llvm::Value *CodeGenFunction::EvaluateExprAsBool(const
Expr *E) {
Loc);
}
+// Helper method to check if the underlying ABI is AAPCS
+static bool isAAPCS(const TargetInfo &TargetInfo) {
+ return TargetInfo.getABI().startswith("aapcs");
+}
+
/// EmitIgnoredExpr - Emit code to compute the specified expression,
/// ignoring the result.
void CodeGenFunction::EmitIgnoredExpr(const Expr *E) {
@@ -4052,15 +4057,120 @@ static bool hasAnyVptr(const QualType Type, const
ASTContext &Context) {
return false;
}
+// AAPCS requires volatile bitfield accesses to be performed using the
+// natural alignment / width of the bitfield declarative type, if that
+// won't cause overlap over a non-bitfield member nor access outside the
+// the data structure.
+bool CodeGenFunction::AdjustAAPCSBitfieldLValue(Address &Base,
+CGBitFieldInfo &Info,
+const FieldDecl *Field,
+const QualType FieldType,
+const CGRecordLayout &RL) {
+ llvm::Type *ResLTy = ConvertTypeForMem(FieldType);
+ // CGRecordLowering::setBitFieldInfo() pre-adjusts the bitfield offsets for
+ // big-endian targets, but it assumes a container of width Info.StorageSize.
+ // Since AAPCS uses a
diff erent container size (width of the type), we first
+ // undo that calculation here and redo it once the bitfield offset within the
+ // new container is calculated
+ const bool BE = CGM.getTypes().getDataLayout().isBigEndian();
+ const unsigned OldOffset =
+ BE ? Info.StorageSize - (Info.Offset + Info.Size) : Info.Offset;
+ // Offset to the bitfield from the beginning of the struct
+ const unsigned AbsoluteOffset =
+ getContext().toBits(Info.StorageOffset) + OldOffset;
+
+ // Container size is the width of the bitfield type
+ const unsigned ContainerSize = ResLTy->getPrimitiveSizeInBits();
+ // Nothing to do if the access uses the desired
+ // container width and is naturally aligned
+ if (Info.StorageSize == ContainerSize && (OldOffset % ContainerSize == 0))
+return
[clang] 2147703 - Revert "[ARM] Follow AACPS standard for volatile bit-fields access width"
Author: Diogo Sampaio
Date: 2020-01-21T15:31:33Z
New Revision: 2147703bde1e1a7a1b89ccb66f55d36fd17620f1
URL:
https://github.com/llvm/llvm-project/commit/2147703bde1e1a7a1b89ccb66f55d36fd17620f1
DIFF:
https://github.com/llvm/llvm-project/commit/2147703bde1e1a7a1b89ccb66f55d36fd17620f1.diff
LOG: Revert "[ARM] Follow AACPS standard for volatile bit-fields access width"
This reverts commit 6a24339a45246b66bd3de88cc9c6a5b5e77c0645.
Submitted using ide button by mistake
Added:
Modified:
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGValue.h
clang/lib/CodeGen/CodeGenFunction.h
clang/test/CodeGen/aapcs-bitfield.c
Removed:
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index c4029c72dd5f..8e0604181fb1 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -177,11 +177,6 @@ llvm::Value *CodeGenFunction::EvaluateExprAsBool(const
Expr *E) {
Loc);
}
-// Helper method to check if the underlying ABI is AAPCS
-static bool isAAPCS(const TargetInfo &TargetInfo) {
- return TargetInfo.getABI().startswith("aapcs");
-}
-
/// EmitIgnoredExpr - Emit code to compute the specified expression,
/// ignoring the result.
void CodeGenFunction::EmitIgnoredExpr(const Expr *E) {
@@ -4057,120 +4052,15 @@ static bool hasAnyVptr(const QualType Type, const
ASTContext &Context) {
return false;
}
-// AAPCS requires volatile bitfield accesses to be performed using the
-// natural alignment / width of the bitfield declarative type, if that
-// won't cause overlap over a non-bitfield member nor access outside the
-// the data structure.
-bool CodeGenFunction::AdjustAAPCSBitfieldLValue(Address &Base,
-CGBitFieldInfo &Info,
-const FieldDecl *Field,
-const QualType FieldType,
-const CGRecordLayout &RL) {
- llvm::Type *ResLTy = ConvertTypeForMem(FieldType);
- // CGRecordLowering::setBitFieldInfo() pre-adjusts the bitfield offsets for
- // big-endian targets, but it assumes a container of width Info.StorageSize.
- // Since AAPCS uses a
diff erent container size (width of the type), we first
- // undo that calculation here and redo it once the bitfield offset within the
- // new container is calculated
- const bool BE = CGM.getTypes().getDataLayout().isBigEndian();
- const unsigned OldOffset =
- BE ? Info.StorageSize - (Info.Offset + Info.Size) : Info.Offset;
- // Offset to the bitfield from the beginning of the struct
- const unsigned AbsoluteOffset =
- getContext().toBits(Info.StorageOffset) + OldOffset;
-
- // Container size is the width of the bitfield type
- const unsigned ContainerSize = ResLTy->getPrimitiveSizeInBits();
- // Nothing to do if the access uses the desired
- // container width and is naturally aligned
- if (Info.StorageSize == ContainerSize && (OldOffset % ContainerSize == 0))
-return false;
-
- // Offset within the container
- unsigned MemberOffset = AbsoluteOffset & (ContainerSize - 1);
-
- // Bail out if an aligned load of the container cannot cover the entire
- // bitfield. This can happen for example, if the bitfield is part of a packed
- // struct. AAPCS does not define access rules for such cases, we let clang to
- // follow its own rules.
- if (MemberOffset + Info.Size > ContainerSize) {
-return false;
- }
- // Re-adjust offsets for big-endian targets
- if (BE)
-MemberOffset = ContainerSize - (MemberOffset + Info.Size);
-
- const CharUnits NewOffset =
- getContext().toCharUnitsFromBits(AbsoluteOffset & ~(ContainerSize - 1));
- const CharUnits End = NewOffset +
-getContext().toCharUnitsFromBits(ContainerSize) -
-CharUnits::One();
-
- const ASTRecordLayout &Layout =
- getContext().getASTRecordLayout(Field->getParent());
- // If we access outside memory outside the record, than bail out
- const CharUnits RecordSize = Layout.getSize();
- if (End >= RecordSize) {
-return false;
- }
-
- // Bail out if performing this load would access non-bitfields members
-
- for (auto it : Field->getParent()->fields()) {
-const FieldDecl &F = *it;
-// We distinct allow bitfields overlaps
-if (F.isBitField())
- continue;
-const CharUnits FOffset = getContext().toCharUnitsFromBits(
-Layout.getFieldOffset(F.getFieldIndex()));
-const CharUnits FEnd =
-FOffset +
-getContext().toCharUnitsFromBits(
-ConvertTypeForMem(F.getType())->getPrimitiveSizeInBits()) -
-CharUnits::One();
-if (End < FOffset) {
- // The other field starts after the desired load end.
- break;
-}
-if (FEnd < NewOffset) {
- // The other field ends before the des
