[PATCH] D77542: [PowerPC] Treat 'Z' inline asm constraint as a true memory constraint
This revision was automatically updated to reflect the committed changes.
Closed by commit rGaede24ecaa08: [PowerPC] Treat Z inline asm
constraint as a true memory constraint (authored by nemanjai).
Changed prior to commit:
https://reviews.llvm.org/D77542?vs=255291=265730#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77542/new/
https://reviews.llvm.org/D77542
Files:
clang/lib/Basic/Targets/PPC.h
clang/test/CodeGen/ppc64-inline-asm.c
Index: clang/test/CodeGen/ppc64-inline-asm.c
===
--- clang/test/CodeGen/ppc64-inline-asm.c
+++ clang/test/CodeGen/ppc64-inline-asm.c
@@ -37,3 +37,16 @@
// CHECK-LABEL: double @test_fmax(double %x, double %y)
// CHECK: call double asm "xsmaxdp ${0:x}, ${1:x}, ${2:x}",
"=^ws,^ws,^ws"(double %x, double %y)
}
+
+void testZ(void *addr) {
+ asm volatile ("dcbz %y0\n" :: "Z"(*(unsigned char *)addr) : "memory");
+// CHECK-LABEL: void @testZ(i8* %addr)
+// CHECK: call void asm sideeffect "dcbz ${0:y}\0A", "*Z,~{memory}"(i8* %addr)
+}
+
+void testZwOff(void *addr, long long off) {
+ asm volatile ("dcbz %y0\n" :: "Z"(*(unsigned char *)(addr + off)) :
"memory");
+// CHECK-LABEL: void @testZwOff(i8* %addr, i64 %off)
+// CHECK: %[[VAL:[^ ]+]] = getelementptr i8, i8* %addr, i64 %off
+// CHECK: call void asm sideeffect "dcbz ${0:y}\0A", "*Z,~{memory}"(i8*
%[[VAL]])
+}
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -276,11 +276,12 @@
break;
case 'Q': // Memory operand that is an offset from a register (it is
// usually better to use `m' or `es' in asm statements)
+ Info.setAllowsRegister();
+ LLVM_FALLTHROUGH;
case 'Z': // Memory operand that is an indexed or indirect from a
// register (it is usually better to use `m' or `es' in
// asm statements)
Info.setAllowsMemory();
- Info.setAllowsRegister();
break;
case 'R': // AIX TOC entry
case 'a': // Address operand that is an indexed or indirect from a
Index: clang/test/CodeGen/ppc64-inline-asm.c
===
--- clang/test/CodeGen/ppc64-inline-asm.c
+++ clang/test/CodeGen/ppc64-inline-asm.c
@@ -37,3 +37,16 @@
// CHECK-LABEL: double @test_fmax(double %x, double %y)
// CHECK: call double asm "xsmaxdp ${0:x}, ${1:x}, ${2:x}", "=^ws,^ws,^ws"(double %x, double %y)
}
+
+void testZ(void *addr) {
+ asm volatile ("dcbz %y0\n" :: "Z"(*(unsigned char *)addr) : "memory");
+// CHECK-LABEL: void @testZ(i8* %addr)
+// CHECK: call void asm sideeffect "dcbz ${0:y}\0A", "*Z,~{memory}"(i8* %addr)
+}
+
+void testZwOff(void *addr, long long off) {
+ asm volatile ("dcbz %y0\n" :: "Z"(*(unsigned char *)(addr + off)) : "memory");
+// CHECK-LABEL: void @testZwOff(i8* %addr, i64 %off)
+// CHECK: %[[VAL:[^ ]+]] = getelementptr i8, i8* %addr, i64 %off
+// CHECK: call void asm sideeffect "dcbz ${0:y}\0A", "*Z,~{memory}"(i8* %[[VAL]])
+}
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -276,11 +276,12 @@
break;
case 'Q': // Memory operand that is an offset from a register (it is
// usually better to use `m' or `es' in asm statements)
+ Info.setAllowsRegister();
+ LLVM_FALLTHROUGH;
case 'Z': // Memory operand that is an indexed or indirect from a
// register (it is usually better to use `m' or `es' in
// asm statements)
Info.setAllowsMemory();
- Info.setAllowsRegister();
break;
case 'R': // AIX TOC entry
case 'a': // Address operand that is an indexed or indirect from a
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77542: [PowerPC] Treat 'Z' inline asm constraint as a true memory constraint
nemanjai marked an inline comment as done.
nemanjai added inline comments.
Comment at: clang/test/CodeGen/ppc64-inline-asm.c:50
+// CHECK-LABEL: void @testZwOff(i8* %addr, i64 %off)
+// CHEC: %[[VAL:[^ ]+]] = getelementptr i8, i8* %addr, i64 %off
+// CHEC: call void asm sideeffect "dcbz ${0:y}\0A", "*Z,~{memory}"(i8*
%[[VAL]])
amyk wrote:
> Missing a `k` in `CHECK`?
Great catch! Thank you.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77542/new/
https://reviews.llvm.org/D77542
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77542: [PowerPC] Treat 'Z' inline asm constraint as a true memory constraint
stefanp accepted this revision as: stefanp. stefanp added a comment. Other than the two missing `K`s in the test case as Amy pointed out LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77542/new/ https://reviews.llvm.org/D77542 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77542: [PowerPC] Treat 'Z' inline asm constraint as a true memory constraint
amyk added inline comments.
Comment at: clang/lib/Basic/Targets/PPC.h:277
break;
case 'Q': // Memory operand that is an offset from a register (it is
// usually better to use `m' or `es' in asm statements)
nemanjai wrote:
> amyk wrote:
> > Just curious, but does this case still require `Info.setAllowsMemory();` as
> > well?
> I don't want to change the behaviour of a QPX-specific asm constraint, so I'd
> rather leave it as-is. `Q` will set both, `Z` will only set "memory".
I see. Thanks for explaining.
Comment at: clang/test/CodeGen/ppc64-inline-asm.c:50
+// CHECK-LABEL: void @testZwOff(i8* %addr, i64 %off)
+// CHEC: %[[VAL:[^ ]+]] = getelementptr i8, i8* %addr, i64 %off
+// CHEC: call void asm sideeffect "dcbz ${0:y}\0A", "*Z,~{memory}"(i8*
%[[VAL]])
Missing a `k` in `CHECK`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77542/new/
https://reviews.llvm.org/D77542
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77542: [PowerPC] Treat 'Z' inline asm constraint as a true memory constraint
lei accepted this revision as: lei. lei added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77542/new/ https://reviews.llvm.org/D77542 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77542: [PowerPC] Treat 'Z' inline asm constraint as a true memory constraint
nemanjai marked an inline comment as done. nemanjai added inline comments. Comment at: clang/lib/Basic/Targets/PPC.h:277 break; case 'Q': // Memory operand that is an offset from a register (it is // usually better to use `m' or `es' in asm statements) amyk wrote: > Just curious, but does this case still require `Info.setAllowsMemory();` as > well? I don't want to change the behaviour of a QPX-specific asm constraint, so I'd rather leave it as-is. `Q` will set both, `Z` will only set "memory". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77542/new/ https://reviews.llvm.org/D77542 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77542: [PowerPC] Treat 'Z' inline asm constraint as a true memory constraint
amyk added inline comments. Comment at: clang/lib/Basic/Targets/PPC.h:277 break; case 'Q': // Memory operand that is an offset from a register (it is // usually better to use `m' or `es' in asm statements) Just curious, but does this case still require `Info.setAllowsMemory();` as well? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77542/new/ https://reviews.llvm.org/D77542 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77542: [PowerPC] Treat 'Z' inline asm constraint as a true memory constraint
nemanjai created this revision.
nemanjai added reviewers: hfinkel, PowerPC.
Herald added subscribers: cfe-commits, shchenz, kbarton.
Herald added a project: clang.
We currently emit incorrect codegen for this constraint because we set it as a
constraint that allows registers. This will cause the value to be copied to the
stack and that address to be passed as the address. This is not what we want.
Fixes: https://bugs.llvm.org/show_bug.cgi?id=42762
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D77542
Files:
clang/lib/Basic/Targets/PPC.h
clang/test/CodeGen/ppc64-inline-asm.c
Index: clang/test/CodeGen/ppc64-inline-asm.c
===
--- clang/test/CodeGen/ppc64-inline-asm.c
+++ clang/test/CodeGen/ppc64-inline-asm.c
@@ -37,3 +37,16 @@
// CHECK-LABEL: double @test_fmax(double %x, double %y)
// CHECK: call double asm "xsmaxdp ${0:x}, ${1:x}, ${2:x}",
"=^ws,^ws,^ws"(double %x, double %y)
}
+
+void testZ(void *addr) {
+ asm volatile ("dcbz %y0\n" :: "Z"(*(unsigned char *)addr) : "memory");
+// CHECK-LABEL: void @testZ(i8* %addr)
+// CHECK: call void asm sideeffect "dcbz ${0:y}\0A", "*Z,~{memory}"(i8* %addr)
+}
+
+void testZwOff(void *addr, long long off) {
+ asm volatile ("dcbz %y0\n" :: "Z"(*(unsigned char *)(addr + off)) :
"memory");
+// CHECK-LABEL: void @testZwOff(i8* %addr, i64 %off)
+// CHEC: %[[VAL:[^ ]+]] = getelementptr i8, i8* %addr, i64 %off
+// CHEC: call void asm sideeffect "dcbz ${0:y}\0A", "*Z,~{memory}"(i8*
%[[VAL]])
+}
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -276,11 +276,12 @@
break;
case 'Q': // Memory operand that is an offset from a register (it is
// usually better to use `m' or `es' in asm statements)
+ Info.setAllowsRegister();
+ LLVM_FALLTHROUGH;
case 'Z': // Memory operand that is an indexed or indirect from a
// register (it is usually better to use `m' or `es' in
// asm statements)
Info.setAllowsMemory();
- Info.setAllowsRegister();
break;
case 'R': // AIX TOC entry
case 'a': // Address operand that is an indexed or indirect from a
Index: clang/test/CodeGen/ppc64-inline-asm.c
===
--- clang/test/CodeGen/ppc64-inline-asm.c
+++ clang/test/CodeGen/ppc64-inline-asm.c
@@ -37,3 +37,16 @@
// CHECK-LABEL: double @test_fmax(double %x, double %y)
// CHECK: call double asm "xsmaxdp ${0:x}, ${1:x}, ${2:x}", "=^ws,^ws,^ws"(double %x, double %y)
}
+
+void testZ(void *addr) {
+ asm volatile ("dcbz %y0\n" :: "Z"(*(unsigned char *)addr) : "memory");
+// CHECK-LABEL: void @testZ(i8* %addr)
+// CHECK: call void asm sideeffect "dcbz ${0:y}\0A", "*Z,~{memory}"(i8* %addr)
+}
+
+void testZwOff(void *addr, long long off) {
+ asm volatile ("dcbz %y0\n" :: "Z"(*(unsigned char *)(addr + off)) : "memory");
+// CHECK-LABEL: void @testZwOff(i8* %addr, i64 %off)
+// CHEC: %[[VAL:[^ ]+]] = getelementptr i8, i8* %addr, i64 %off
+// CHEC: call void asm sideeffect "dcbz ${0:y}\0A", "*Z,~{memory}"(i8* %[[VAL]])
+}
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -276,11 +276,12 @@
break;
case 'Q': // Memory operand that is an offset from a register (it is
// usually better to use `m' or `es' in asm statements)
+ Info.setAllowsRegister();
+ LLVM_FALLTHROUGH;
case 'Z': // Memory operand that is an indexed or indirect from a
// register (it is usually better to use `m' or `es' in
// asm statements)
Info.setAllowsMemory();
- Info.setAllowsRegister();
break;
case 'R': // AIX TOC entry
case 'a': // Address operand that is an indexed or indirect from a
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
