[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
lkail updated this revision to Diff 441293. lkail added a comment. Option name changed to `-mabi=quadword-atomics` as nemanja suggested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127189/new/ https://reviews.llvm.org/D127189 Files: clang/include/clang/Basic/LangOptions.def clang/include/clang/Driver/Options.td clang/lib/Basic/Targets/PPC.cpp clang/lib/Driver/ToolChains/Clang.cpp clang/lib/Frontend/CompilerInvocation.cpp clang/test/CodeGen/PowerPC/quadword-atomics.c clang/test/Driver/aix-quadword-atomics-abi.c clang/test/Driver/ppc-unsupported.c clang/test/Sema/atomic-ops.c Index: clang/test/Sema/atomic-ops.c === --- clang/test/Sema/atomic-ops.c +++ clang/test/Sema/atomic-ops.c @@ -10,6 +10,12 @@ // RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \ // RUN: -fsyntax-only -triple=powerpc64le-linux-gnu -std=c11 \ // RUN: -target-cpu pwr8 -DPPC64_PWR8 +// RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \ +// RUN: -fsyntax-only -triple=powerpc64-unknown-aix -std=c11 \ +// RUN: -target-cpu pwr8 +// RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \ +// RUN: -fsyntax-only -triple=powerpc64-unknown-aix -std=c11 \ +// RUN: -mabi=quadword-atomics -target-cpu pwr8 -DPPC64_PWR8 // Basic parsing/Sema tests for __c11_atomic_* Index: clang/test/Driver/ppc-unsupported.c === --- clang/test/Driver/ppc-unsupported.c +++ clang/test/Driver/ppc-unsupported.c @@ -7,4 +7,14 @@ // RUN: -c %s 2>&1 | FileCheck %s // RUN: not %clang -target powerpc64le-unknown-linux -msvr4-struct-return \ // RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc64-unknown-freebsd -mabi=quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc64-unknown-linux -mabi=quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc64le-unknown-linux -mabi=quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc-unknown-unknown -mabi=quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc-unknown-aix -mabi=quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s // CHECK: unsupported option Index: clang/test/Driver/aix-quadword-atomics-abi.c === --- /dev/null +++ clang/test/Driver/aix-quadword-atomics-abi.c @@ -0,0 +1,11 @@ +// RUN: %clang -### -target powerpc-unknown-aix -S %s 2>&1 | FileCheck %s +// RUN: %clang -### -target powerpc64-unknown-aix -S %s 2>&1 | FileCheck %s +// RUN: %clang -### -target powerpc-unknown-aix -mabi=quadword-atomics -S \ +// RUN:%s 2>&1 | FileCheck --check-prefix=CHECK-UNSUPPORTED-TARGET %s +// RUN: %clang -### -target powerpc64-unknown-aix -mabi=quadword-atomics -S \ +// RUN:%s 2>&1 | FileCheck %s --check-prefix=CHECK-QUADWORD-ATOMICS +// +// CHECK-UNSUPPORTED-TARGET: unsupported option '-mabi=quadword-atomics' for target 'powerpc-unknown-aix' +// CHECK-NOT: "-mabi=quadword-atomics" +// CHECK-QUADWORD-ATOMICS: "-cc1" +// CHECK-QUADWORD-ATOMICS-SAME: "-mabi=quadword-atomics" Index: clang/test/CodeGen/PowerPC/quadword-atomics.c === --- clang/test/CodeGen/PowerPC/quadword-atomics.c +++ clang/test/CodeGen/PowerPC/quadword-atomics.c @@ -1,9 +1,15 @@ // RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64le-linux-gnu \ -// RUN: -target-cpu pwr8 -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64-PWR8 +// RUN: -target-cpu pwr8 -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64-QUADWORD-ATOMICS // RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64le-linux-gnu \ // RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64 // RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64-unknown-aix \ // RUN: -target-cpu pwr7 -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64 +// RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64-unknown-aix \ +// RUN: -target-cpu pwr8 -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64 +// RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64-unknown-aix \ +// RUN: -mabi=quadword-atomics -target-cpu pwr8 -emit-llvm -o - %s | FileCheck %s \ +// RUN: --check-prefix=PPC64-QUADWORD-ATOMICS + typedef struct { char x[16]; @@ -13,8 +19,8 @@ typedef __int128_t int128_t; -// PPC64-PWR8-LABEL: @test_load( -// PPC64-PWR8:[[TMP3:%.*]] = load atomic i128, i128* [[TMP1:%.*]] acquire, align 16 +// PPC64-QUADWORD-ATOMICS-LABEL: @test_load( +// PPC64-QUADWORD-ATOMICS:[[TMP3:%.*]] = load atomic i128, i128* [[TMP1:%.*]] acquire, align 16 // // PPC64-LABEL: @test_load( // PPC
[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
lkail added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127189/new/ https://reviews.llvm.org/D127189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
lkail added a comment. Gentle ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127189/new/ https://reviews.llvm.org/D127189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
shchenz added a comment. Can we use the feature bit `FeatureQuadwordAtomic` to decide whether QuadAtomic is supported or not on AIX? Like what we do for Linux. The reason we need this option is: we may need to compile a lock free libatomic on a Power7 or below target? If so, do we have similar issue on Linux? Thanks. Comment at: clang/include/clang/Driver/Options.td:3611 HelpText<"Enable the default Altivec ABI on AIX (AIX only). Uses only volatile vector registers.">; +def maix_quadword_atomics : Flag<["-"], "maix64-quadword-atomics">, + Group, Flags<[CC1Option]>, amyk wrote: > Would it be better if we called this `maix64-quadword-atomics` instead? Do we need to change the backend check below too? ``` bool PPCTargetLowering::shouldInlineQuadwordAtomics() const { // TODO: 16-byte atomic type support for AIX is in progress; we should be able // to inline 16-byte atomic ops on AIX too in the future. return Subtarget.isPPC64() && (EnableQuadwordAtomics || !Subtarget.getTargetTriple().isOSAIX()) && Subtarget.hasQuadwordAtomics(); } ``` Comment at: clang/lib/Basic/Targets/PPC.cpp:854 + HasQuadwordAtomics) +MaxAtomicInlineWidth = 128; } Can we set `MaxAtomicInlineWidth` in `PPC64TargetInfo::setMaxAtomicWidth()`? There is a `TODO` there Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127189/new/ https://reviews.llvm.org/D127189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
lkail added a comment. > Can we use the feature bit FeatureQuadwordAtomic to decide whether QuadAtomic > is supported or not on AIX? Like what we do for Linux. `FeatureQuadwordAtomic` is for cpu level control, while `-mabi=quadword-atomics` is for ABI level. AIX running on pwr8+ also features `FeatureQuadwordAtomic`, but in the case described in the patch summary, sometimes we can't enable quadword lock free atomics on AIX by default, so that clang generate libcalls into libatomic rather than inlining lock free operations. libatomic has the final decision to use lock-free version or not. > The reason we need this option is: we may need to compile a lock free > libatomic on a Power7 or below target? We need to compile a quadword lock free libatomic for CPUs above pwr8. > If so, do we have similar issue on Linux? Thanks. On Linux, `clang` is linking against GNU's libatomic by default, that depends on GNU libatomic's behaviour. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127189/new/ https://reviews.llvm.org/D127189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
lkail added inline comments. Comment at: clang/include/clang/Driver/Options.td:3611 HelpText<"Enable the default Altivec ABI on AIX (AIX only). Uses only volatile vector registers.">; +def maix_quadword_atomics : Flag<["-"], "maix64-quadword-atomics">, + Group, Flags<[CC1Option]>, shchenz wrote: > amyk wrote: > > Would it be better if we called this `maix64-quadword-atomics` instead? > Do we need to change the backend check below too? > ``` > bool PPCTargetLowering::shouldInlineQuadwordAtomics() const { > // TODO: 16-byte atomic type support for AIX is in progress; we should be > able > // to inline 16-byte atomic ops on AIX too in the future. > return Subtarget.isPPC64() && > (EnableQuadwordAtomics || !Subtarget.getTargetTriple().isOSAIX()) && > Subtarget.hasQuadwordAtomics(); > } > ``` We don't need to change this yet. When we are compiling a quadword lock free libatomic, we use options `-mabi=quadword-atomics -mllvm -ppc-quadword-atomics` to enforce generating quadword lock-free code on AIX. Comment at: clang/lib/Basic/Targets/PPC.cpp:854 + HasQuadwordAtomics) +MaxAtomicInlineWidth = 128; } shchenz wrote: > Can we set `MaxAtomicInlineWidth` in `PPC64TargetInfo::setMaxAtomicWidth()`? > There is a `TODO` there The `TODO` marks our roadmap towards enabling quardword lock free atomics on AIX too. Putting adjustment here is implementation reason: we don't context of `LanguageOptions` in `PPC64TargetInfo::PPC64TargetInfo`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127189/new/ https://reviews.llvm.org/D127189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
shchenz added a comment. Thanks for the explanation. I am more clear now about the background. Comment at: clang/include/clang/Driver/Options.td:3611 HelpText<"Enable the default Altivec ABI on AIX (AIX only). Uses only volatile vector registers.">; +def maix_quadword_atomics : Flag<["-"], "maix64-quadword-atomics">, + Group, Flags<[CC1Option]>, lkail wrote: > shchenz wrote: > > amyk wrote: > > > Would it be better if we called this `maix64-quadword-atomics` instead? > > Do we need to change the backend check below too? > > ``` > > bool PPCTargetLowering::shouldInlineQuadwordAtomics() const { > > // TODO: 16-byte atomic type support for AIX is in progress; we should be > > able > > // to inline 16-byte atomic ops on AIX too in the future. > > return Subtarget.isPPC64() && > > (EnableQuadwordAtomics || !Subtarget.getTargetTriple().isOSAIX()) > > && > > Subtarget.hasQuadwordAtomics(); > > } > > ``` > We don't need to change this yet. When we are compiling a quadword lock free > libatomic, we use options `-mabi=quadword-atomics -mllvm > -ppc-quadword-atomics` to enforce generating quadword lock-free code on AIX. This makes me confuse. We need to two different options to control the frontend and backend behavior? Is it the final usage? Or we will add a follow up patch to map the backend one to the FE one? IMO finally we only need the driver option `-mabi=quadword-atomics` to control the final code generation for 128 bit atomic operations, right? Comment at: clang/lib/Basic/Targets/PPC.cpp:854 + HasQuadwordAtomics) +MaxAtomicInlineWidth = 128; } lkail wrote: > shchenz wrote: > > Can we set `MaxAtomicInlineWidth` in > > `PPC64TargetInfo::setMaxAtomicWidth()`? There is a `TODO` there > The `TODO` marks our roadmap towards enabling quardword lock free atomics on > AIX too. Putting adjustment here is implementation reason: we don't context > of `LanguageOptions` in `PPC64TargetInfo::PPC64TargetInfo`. OK, fair enough. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127189/new/ https://reviews.llvm.org/D127189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
lkail added inline comments. Comment at: clang/include/clang/Driver/Options.td:3611 HelpText<"Enable the default Altivec ABI on AIX (AIX only). Uses only volatile vector registers.">; +def maix_quadword_atomics : Flag<["-"], "maix64-quadword-atomics">, + Group, Flags<[CC1Option]>, shchenz wrote: > lkail wrote: > > shchenz wrote: > > > amyk wrote: > > > > Would it be better if we called this `maix64-quadword-atomics` instead? > > > Do we need to change the backend check below too? > > > ``` > > > bool PPCTargetLowering::shouldInlineQuadwordAtomics() const { > > > // TODO: 16-byte atomic type support for AIX is in progress; we should > > > be able > > > // to inline 16-byte atomic ops on AIX too in the future. > > > return Subtarget.isPPC64() && > > > (EnableQuadwordAtomics || > > > !Subtarget.getTargetTriple().isOSAIX()) && > > > Subtarget.hasQuadwordAtomics(); > > > } > > > ``` > > We don't need to change this yet. When we are compiling a quadword lock > > free libatomic, we use options `-mabi=quadword-atomics -mllvm > > -ppc-quadword-atomics` to enforce generating quadword lock-free code on AIX. > This makes me confuse. We need to two different options to control the > frontend and backend behavior? > > Is it the final usage? Or we will add a follow up patch to map the backend > one to the FE one? IMO finally we only need the driver option > `-mabi=quadword-atomics` to control the final code generation for 128 bit > atomic operations, right? > This makes me confuse. We need to two different options to control the > frontend and backend behavior? This is multi-lang support consideration. clang is not the only frontend we have using LLVM as backend on AIX. If other language frontend generates `store atomic i128, ...`, the backend is supposed to generate libcalls into libatomic currently. > Is it the final usage? No. We finally want to achieve `-mabi=quadword-atomics` by default and generate inline atomic code for cpu above pwr7 by default(no need to take OS into consideration). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127189/new/ https://reviews.llvm.org/D127189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
shchenz added inline comments. Comment at: clang/include/clang/Driver/Options.td:3611 HelpText<"Enable the default Altivec ABI on AIX (AIX only). Uses only volatile vector registers.">; +def maix_quadword_atomics : Flag<["-"], "maix64-quadword-atomics">, + Group, Flags<[CC1Option]>, lkail wrote: > shchenz wrote: > > lkail wrote: > > > shchenz wrote: > > > > amyk wrote: > > > > > Would it be better if we called this `maix64-quadword-atomics` > > > > > instead? > > > > Do we need to change the backend check below too? > > > > ``` > > > > bool PPCTargetLowering::shouldInlineQuadwordAtomics() const { > > > > // TODO: 16-byte atomic type support for AIX is in progress; we > > > > should be able > > > > // to inline 16-byte atomic ops on AIX too in the future. > > > > return Subtarget.isPPC64() && > > > > (EnableQuadwordAtomics || > > > > !Subtarget.getTargetTriple().isOSAIX()) && > > > > Subtarget.hasQuadwordAtomics(); > > > > } > > > > ``` > > > We don't need to change this yet. When we are compiling a quadword lock > > > free libatomic, we use options `-mabi=quadword-atomics -mllvm > > > -ppc-quadword-atomics` to enforce generating quadword lock-free code on > > > AIX. > > This makes me confuse. We need to two different options to control the > > frontend and backend behavior? > > > > Is it the final usage? Or we will add a follow up patch to map the backend > > one to the FE one? IMO finally we only need the driver option > > `-mabi=quadword-atomics` to control the final code generation for 128 bit > > atomic operations, right? > > This makes me confuse. We need to two different options to control the > > frontend and backend behavior? > > This is multi-lang support consideration. clang is not the only frontend we > have using LLVM as backend on AIX. If other language frontend generates > `store atomic i128, ...`, the backend is supposed to generate libcalls into > libatomic currently. > > > Is it the final usage? > No. We finally want to achieve `-mabi=quadword-atomics` by default and > generate inline atomic code for cpu above pwr7 by default(no need to take OS > into consideration). I know what you mean. But I assume the driver option `-mabi=quadword-atomics` will impact the assembly instead of just impact the frontend, right? Using `-mllvm` option is not right as the final solution. There are some driver options example, like `-gstrict-dwarf`, Frontend can control the backend behavior and the backend can also change this option by `-strict-dwarf`. Could you please explain: 1: how the backend will handle `-mabi=quadword-atomics` in future? 2: on what condition, we can start to remove below TODOs: ``` bool PPCTargetLowering::shouldInlineQuadwordAtomics() const { // TODO: 16-byte atomic type support for AIX is in progress; } ``` ``` PPC64TargetInfo::setMaxAtomicWidth() { // TODO: We should allow AIX to inline quadword atomics in the future. } ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127189/new/ https://reviews.llvm.org/D127189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
lkail added inline comments. Comment at: clang/include/clang/Driver/Options.td:3611 HelpText<"Enable the default Altivec ABI on AIX (AIX only). Uses only volatile vector registers.">; +def maix_quadword_atomics : Flag<["-"], "maix64-quadword-atomics">, + Group, Flags<[CC1Option]>, shchenz wrote: > lkail wrote: > > shchenz wrote: > > > lkail wrote: > > > > shchenz wrote: > > > > > amyk wrote: > > > > > > Would it be better if we called this `maix64-quadword-atomics` > > > > > > instead? > > > > > Do we need to change the backend check below too? > > > > > ``` > > > > > bool PPCTargetLowering::shouldInlineQuadwordAtomics() const { > > > > > // TODO: 16-byte atomic type support for AIX is in progress; we > > > > > should be able > > > > > // to inline 16-byte atomic ops on AIX too in the future. > > > > > return Subtarget.isPPC64() && > > > > > (EnableQuadwordAtomics || > > > > > !Subtarget.getTargetTriple().isOSAIX()) && > > > > > Subtarget.hasQuadwordAtomics(); > > > > > } > > > > > ``` > > > > We don't need to change this yet. When we are compiling a quadword lock > > > > free libatomic, we use options `-mabi=quadword-atomics -mllvm > > > > -ppc-quadword-atomics` to enforce generating quadword lock-free code on > > > > AIX. > > > This makes me confuse. We need to two different options to control the > > > frontend and backend behavior? > > > > > > Is it the final usage? Or we will add a follow up patch to map the > > > backend one to the FE one? IMO finally we only need the driver option > > > `-mabi=quadword-atomics` to control the final code generation for 128 bit > > > atomic operations, right? > > > This makes me confuse. We need to two different options to control the > > > frontend and backend behavior? > > > > This is multi-lang support consideration. clang is not the only frontend we > > have using LLVM as backend on AIX. If other language frontend generates > > `store atomic i128, ...`, the backend is supposed to generate libcalls into > > libatomic currently. > > > > > Is it the final usage? > > No. We finally want to achieve `-mabi=quadword-atomics` by default and > > generate inline atomic code for cpu above pwr7 by default(no need to take > > OS into consideration). > I know what you mean. But I assume the driver option `-mabi=quadword-atomics` > will impact the assembly instead of just impact the frontend, right? Using > `-mllvm` option is not right as the final solution. > > There are some driver options example, like `-gstrict-dwarf`, Frontend can > control the backend behavior and the backend can also change this option by > `-strict-dwarf`. > > Could you please explain: > 1: how the backend will handle `-mabi=quadword-atomics` in future? > 2: on what condition, we can start to remove below TODOs: > ``` > bool PPCTargetLowering::shouldInlineQuadwordAtomics() const { > // TODO: 16-byte atomic type support for AIX is in progress; > } > ``` > > ``` > PPC64TargetInfo::setMaxAtomicWidth() { > // TODO: We should allow AIX to inline quadword atomics in the future. > } > ``` 1, 2 can be answered together. After we ship new libatomic to users(I assume that that isn't in near future, since this requires AIX OS upgrade), we can enable quadword lock free atomics in both clang and llvm backend by default. Backend isn't aware of `-mabi=quadword-atomics`. This option changes layout of quadword atomic types(align to 16 byte), and generate atomic LLVM IR rather than generating libcalls in LLVM IR. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127189/new/ https://reviews.llvm.org/D127189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
shchenz accepted this revision as: shchenz. shchenz added a comment. This revision is now accepted and ready to land. LGTM. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127189/new/ https://reviews.llvm.org/D127189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1cbaf681b0f1: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX (authored by lkail). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127189/new/ https://reviews.llvm.org/D127189 Files: clang/include/clang/Basic/LangOptions.def clang/include/clang/Driver/Options.td clang/lib/Basic/Targets/PPC.cpp clang/lib/Driver/ToolChains/Clang.cpp clang/lib/Frontend/CompilerInvocation.cpp clang/test/CodeGen/PowerPC/quadword-atomics.c clang/test/Driver/aix-quadword-atomics-abi.c clang/test/Driver/ppc-unsupported.c clang/test/Sema/atomic-ops.c Index: clang/test/Sema/atomic-ops.c === --- clang/test/Sema/atomic-ops.c +++ clang/test/Sema/atomic-ops.c @@ -10,6 +10,12 @@ // RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \ // RUN: -fsyntax-only -triple=powerpc64le-linux-gnu -std=c11 \ // RUN: -target-cpu pwr8 -DPPC64_PWR8 +// RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \ +// RUN: -fsyntax-only -triple=powerpc64-unknown-aix -std=c11 \ +// RUN: -target-cpu pwr8 +// RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \ +// RUN: -fsyntax-only -triple=powerpc64-unknown-aix -std=c11 \ +// RUN: -mabi=quadword-atomics -target-cpu pwr8 -DPPC64_PWR8 // Basic parsing/Sema tests for __c11_atomic_* Index: clang/test/Driver/ppc-unsupported.c === --- clang/test/Driver/ppc-unsupported.c +++ clang/test/Driver/ppc-unsupported.c @@ -7,4 +7,14 @@ // RUN: -c %s 2>&1 | FileCheck %s // RUN: not %clang -target powerpc64le-unknown-linux -msvr4-struct-return \ // RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc64-unknown-freebsd -mabi=quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc64-unknown-linux -mabi=quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc64le-unknown-linux -mabi=quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc-unknown-unknown -mabi=quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc-unknown-aix -mabi=quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s // CHECK: unsupported option Index: clang/test/Driver/aix-quadword-atomics-abi.c === --- /dev/null +++ clang/test/Driver/aix-quadword-atomics-abi.c @@ -0,0 +1,11 @@ +// RUN: %clang -### -target powerpc-unknown-aix -S %s 2>&1 | FileCheck %s +// RUN: %clang -### -target powerpc64-unknown-aix -S %s 2>&1 | FileCheck %s +// RUN: %clang -### -target powerpc-unknown-aix -mabi=quadword-atomics -S \ +// RUN:%s 2>&1 | FileCheck --check-prefix=CHECK-UNSUPPORTED-TARGET %s +// RUN: %clang -### -target powerpc64-unknown-aix -mabi=quadword-atomics -S \ +// RUN:%s 2>&1 | FileCheck %s --check-prefix=CHECK-QUADWORD-ATOMICS +// +// CHECK-UNSUPPORTED-TARGET: unsupported option '-mabi=quadword-atomics' for target 'powerpc-unknown-aix' +// CHECK-NOT: "-mabi=quadword-atomics" +// CHECK-QUADWORD-ATOMICS: "-cc1" +// CHECK-QUADWORD-ATOMICS-SAME: "-mabi=quadword-atomics" Index: clang/test/CodeGen/PowerPC/quadword-atomics.c === --- clang/test/CodeGen/PowerPC/quadword-atomics.c +++ clang/test/CodeGen/PowerPC/quadword-atomics.c @@ -1,9 +1,15 @@ // RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64le-linux-gnu \ -// RUN: -target-cpu pwr8 -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64-PWR8 +// RUN: -target-cpu pwr8 -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64-QUADWORD-ATOMICS // RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64le-linux-gnu \ // RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64 // RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64-unknown-aix \ // RUN: -target-cpu pwr7 -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64 +// RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64-unknown-aix \ +// RUN: -target-cpu pwr8 -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64 +// RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64-unknown-aix \ +// RUN: -mabi=quadword-atomics -target-cpu pwr8 -emit-llvm -o - %s | FileCheck %s \ +// RUN: --check-prefix=PPC64-QUADWORD-ATOMICS + typedef struct { char x[16]; @@ -13,8 +19,8 @@ typedef __int128_t int128_t; -// PPC64-PWR8-LABEL: @test_load( -// PPC64-PWR8:[[TMP3:%.*]] = load atomic i128, i128* [[TMP1:%.*]] acquire, align 16 +// PPC64-QUADWORD-ATOMICS-LABEL: @test_load( +// PPC64-QUADWORD-ATO
[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
lkail added a comment. Gentle ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127189/new/ https://reviews.llvm.org/D127189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
amyk added a comment. This patch makes sense. I had some questions regarding it. Comment at: clang/include/clang/Driver/Options.td:3611 HelpText<"Enable the default Altivec ABI on AIX (AIX only). Uses only volatile vector registers.">; +def maix_quadword_atomics : Flag<["-"], "maix64-quadword-atomics">, + Group, Flags<[CC1Option]>, Would it be better if we called this `maix64-quadword-atomics` instead? Comment at: clang/test/Driver/ppc-unsupported.c:12 +// RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc64le-unknown-linux -maix64-quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s Should we have a big endian Linux check, too? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127189/new/ https://reviews.llvm.org/D127189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
amyk added inline comments. Comment at: clang/test/Driver/ppc-unsupported.c:12 +// RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc64le-unknown-linux -maix64-quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s amyk wrote: > Should we have a big endian Linux check, too? Oh, sorry. I noticed there wasn't `powerpc64-unknown-linux` but I realized I think `powerpc64-unknown-freebsd` is supposed to be the big endian 64-bit Linux check, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127189/new/ https://reviews.llvm.org/D127189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
lkail added inline comments. Comment at: clang/test/Driver/ppc-unsupported.c:12 +// RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc64le-unknown-linux -maix64-quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s amyk wrote: > amyk wrote: > > Should we have a big endian Linux check, too? > Oh, sorry. I noticed there wasn't `powerpc64-unknown-linux` but I realized I > think `powerpc64-unknown-freebsd` is supposed to be the big endian 64-bit > Linux check, right? Added OS check is following lines above. I'm ok to add `powerpc64-unknown-linux` too. Thanks for pointing it out. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127189/new/ https://reviews.llvm.org/D127189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
lkail updated this revision to Diff 440860. lkail added a comment. Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127189/new/ https://reviews.llvm.org/D127189 Files: clang/include/clang/Basic/LangOptions.def clang/include/clang/Driver/Options.td clang/lib/Basic/Targets/PPC.cpp clang/lib/Driver/ToolChains/Clang.cpp clang/lib/Frontend/CompilerInvocation.cpp clang/test/CodeGen/PowerPC/quadword-atomics.c clang/test/Driver/aix-quadword-atomics-abi.c clang/test/Driver/ppc-unsupported.c clang/test/Sema/atomic-ops.c Index: clang/test/Sema/atomic-ops.c === --- clang/test/Sema/atomic-ops.c +++ clang/test/Sema/atomic-ops.c @@ -10,6 +10,12 @@ // RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \ // RUN: -fsyntax-only -triple=powerpc64le-linux-gnu -std=c11 \ // RUN: -target-cpu pwr8 -DPPC64_PWR8 +// RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \ +// RUN: -fsyntax-only -triple=powerpc64-unknown-aix -std=c11 \ +// RUN: -target-cpu pwr8 +// RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \ +// RUN: -fsyntax-only -triple=powerpc64-unknown-aix -std=c11 \ +// RUN: -maix64-quadword-atomics -target-cpu pwr8 -DPPC64_PWR8 // Basic parsing/Sema tests for __c11_atomic_* Index: clang/test/Driver/ppc-unsupported.c === --- clang/test/Driver/ppc-unsupported.c +++ clang/test/Driver/ppc-unsupported.c @@ -7,4 +7,14 @@ // RUN: -c %s 2>&1 | FileCheck %s // RUN: not %clang -target powerpc64le-unknown-linux -msvr4-struct-return \ // RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc64-unknown-freebsd -maix64-quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc64-unknown-linux -maix64-quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc64le-unknown-linux -maix64-quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc-unknown-unknown -maix64-quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc-unknown-aix -maix64-quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s // CHECK: unsupported option Index: clang/test/Driver/aix-quadword-atomics-abi.c === --- /dev/null +++ clang/test/Driver/aix-quadword-atomics-abi.c @@ -0,0 +1,11 @@ +// RUN: %clang -### -target powerpc-unknown-aix -S %s 2>&1 | FileCheck %s +// RUN: %clang -### -target powerpc64-unknown-aix -S %s 2>&1 | FileCheck %s +// RUN: %clang -### -target powerpc-unknown-aix -maix64-quadword-atomics -S \ +// RUN:%s 2>&1 | FileCheck --check-prefix=CHECK-UNSUPPORTED-TARGET %s +// RUN: %clang -### -target powerpc64-unknown-aix -maix64-quadword-atomics -S \ +// RUN:%s 2>&1 | FileCheck %s --check-prefix=CHECK-QUADWORD-ATOMICS +// +// CHECK-UNSUPPORTED-TARGET: unsupported option '-maix64-quadword-atomics' for target 'powerpc-unknown-aix' +// CHECK-NOT: "-maix64-quadword-atomics" +// CHECK-QUADWORD-ATOMICS: "-cc1" +// CHECK-QUADWORD-ATOMICS-SAME: "-maix64-quadword-atomics" Index: clang/test/CodeGen/PowerPC/quadword-atomics.c === --- clang/test/CodeGen/PowerPC/quadword-atomics.c +++ clang/test/CodeGen/PowerPC/quadword-atomics.c @@ -1,9 +1,15 @@ // RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64le-linux-gnu \ -// RUN: -target-cpu pwr8 -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64-PWR8 +// RUN: -target-cpu pwr8 -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64-QUADWORD-ATOMICS // RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64le-linux-gnu \ // RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64 // RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64-unknown-aix \ // RUN: -target-cpu pwr7 -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64 +// RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64-unknown-aix \ +// RUN: -target-cpu pwr8 -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64 +// RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64-unknown-aix \ +// RUN: -maix64-quadword-atomics -target-cpu pwr8 -emit-llvm -o - %s | FileCheck %s \ +// RUN: --check-prefix=PPC64-QUADWORD-ATOMICS + typedef struct { char x[16]; @@ -13,8 +19,8 @@ typedef __int128_t int128_t; -// PPC64-PWR8-LABEL: @test_load( -// PPC64-PWR8:[[TMP3:%.*]] = load atomic i128, i128* [[TMP1:%.*]] acquire, align 16 +// PPC64-QUADWORD-ATOMICS-LABEL: @test_load( +// PPC64-QUADWORD-ATOMICS:[[TMP3:%.*]] = load atomic i128, i128* [[TMP1:%.*]] acquire, align 16 // // PPC64-LABEL: @test_load( // PPC64:call void @__atomic_l
[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
nemanjai added a comment. I am ok with this change overall, I just have a couple of questions about naming of the option. 1. Is there any precedent for options that start with `-maix` or `-m` for any other OS? 2. Is `quadword` the best word to use? There is no type information and this is restricted to integers. Would something like `-maix-i128-atomics` be a better name? 3. Since this is kind of an ABI-related decision, would it make sense (and would it be possible) to make this a further suboption to the `-mabi` option? Something like `-mabi=vec-extabi,i128-atomics,ieeelongdouble` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127189/new/ https://reviews.llvm.org/D127189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
lkail added a comment. > Is there any precedent for options that start with -maix or -m for any > other OS? There is `-maix-struct-return`. > Is quadword the best word to use? There is no type information and this is > restricted to integers. Would something like -maix-i128-atomics be a better > name? 'quadword' is used in ISA manual, so I follow it. Not merely `i128`, some struct types are also included, like struct Q { char c[16]; }; > Since this is kind of an ABI-related decision, would it make sense (and would > it be possible) to make this a further suboption to the -mabi option? This makes sense. I'll try this solution. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127189/new/ https://reviews.llvm.org/D127189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127189: [clang][AIX] Add option to control quadword lock free atomics ABI on AIX
lkail created this revision. lkail added reviewers: hubert.reinterpretcast, cebowleratibm, xingxue, PowerPC. Herald added subscribers: kbarton, nemanjai. Herald added a project: All. lkail requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. We are supporting quadword lock free atomics on AIX. For the situation that users on AIX are using a libatomic that is lock-based for quadword types, we can't enable quadword lock free atomics by default on AIX in case user's new code and legacy code access the same shared atomic quadword variable, we can't guarentee atomicity. So we need an option to enable quadword lock free atomics on AIX, thus we can build a quadword lock-free libatomic for users to make the transition smooth. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D127189 Files: clang/include/clang/Basic/LangOptions.def clang/include/clang/Driver/Options.td clang/lib/Basic/Targets/PPC.cpp clang/lib/Driver/ToolChains/Clang.cpp clang/lib/Frontend/CompilerInvocation.cpp clang/test/CodeGen/PowerPC/quadword-atomics.c clang/test/Driver/aix-quadword-atomics-abi.c clang/test/Driver/ppc-unsupported.c clang/test/Sema/atomic-ops.c Index: clang/test/Sema/atomic-ops.c === --- clang/test/Sema/atomic-ops.c +++ clang/test/Sema/atomic-ops.c @@ -10,6 +10,12 @@ // RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \ // RUN: -fsyntax-only -triple=powerpc64le-linux-gnu -std=c11 \ // RUN: -target-cpu pwr8 -DPPC64_PWR8 +// RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \ +// RUN: -fsyntax-only -triple=powerpc64-unknown-aix -std=c11 \ +// RUN: -target-cpu pwr8 +// RUN: %clang_cc1 %s -verify -fgnuc-version=4.2.1 -ffreestanding \ +// RUN: -fsyntax-only -triple=powerpc64-unknown-aix -std=c11 \ +// RUN: -maix64-quadword-atomics -target-cpu pwr8 -DPPC64_PWR8 // Basic parsing/Sema tests for __c11_atomic_* Index: clang/test/Driver/ppc-unsupported.c === --- clang/test/Driver/ppc-unsupported.c +++ clang/test/Driver/ppc-unsupported.c @@ -7,4 +7,12 @@ // RUN: -c %s 2>&1 | FileCheck %s // RUN: not %clang -target powerpc64le-unknown-linux -msvr4-struct-return \ // RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc64-unknown-freebsd -maix64-quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc64le-unknown-linux -maix64-quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc-unknown-unknown -maix64-quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s +// RUN: not %clang -target powerpc-unknown-aix -maix64-quadword-atomics \ +// RUN: -c %s 2>&1 | FileCheck %s // CHECK: unsupported option Index: clang/test/Driver/aix-quadword-atomics-abi.c === --- /dev/null +++ clang/test/Driver/aix-quadword-atomics-abi.c @@ -0,0 +1,11 @@ +// RUN: %clang -### -target powerpc-unknown-aix -S %s 2>&1 | FileCheck %s +// RUN: %clang -### -target powerpc64-unknown-aix -S %s 2>&1 | FileCheck %s +// RUN: %clang -### -target powerpc-unknown-aix -maix64-quadword-atomics -S \ +// RUN:%s 2>&1 | FileCheck --check-prefix=CHECK-UNSUPPORTED-TARGET %s +// RUN: %clang -### -target powerpc64-unknown-aix -maix64-quadword-atomics -S \ +// RUN:%s 2>&1 | FileCheck %s --check-prefix=CHECK-QUADWORD-ATOMICS +// +// CHECK-UNSUPPORTED-TARGET: unsupported option '-maix64-quadword-atomics' for target 'powerpc-unknown-aix' +// CHECK-NOT: "-maix64-quadword-atomics" +// CHECK-QUADWORD-ATOMICS: "-cc1" +// CHECK-QUADWORD-ATOMICS-SAME: "-maix64-quadword-atomics" Index: clang/test/CodeGen/PowerPC/quadword-atomics.c === --- clang/test/CodeGen/PowerPC/quadword-atomics.c +++ clang/test/CodeGen/PowerPC/quadword-atomics.c @@ -1,9 +1,15 @@ // RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64le-linux-gnu \ -// RUN: -target-cpu pwr8 -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64-PWR8 +// RUN: -target-cpu pwr8 -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64-QUADWORD-ATOMICS // RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64le-linux-gnu \ // RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64 // RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64-unknown-aix \ // RUN: -target-cpu pwr7 -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64 +// RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64-unknown-aix \ +// RUN: -target-cpu pwr8 -emit-llvm -o - %s | FileCheck %s --check-prefix=PPC64 +// RUN: %clang_cc1 -no-opaque-pointers -Werror -Wno-atomic-alignment -triple powerpc64-unknown-aix \ +// RUN: -maix64-quadword-atomics -t