Failure to build librseq on ppc
Mathieu Desnoyers
mathieu.desnoyers at efficios.com
Thu Jul 9 00:00:01 AEST 2020
----- On Jul 8, 2020, at 8:33 AM, Mathieu Desnoyers mathieu.desnoyers at efficios.com wrote:
> ----- On Jul 7, 2020, at 8:59 PM, Segher Boessenkool segher at kernel.crashing.org
> wrote:
[...]
>>
>> So perhaps you have code like
>>
>> int *p;
>> int x;
>> ...
>> asm ("lwz %0,%1" : "=r"(x) : "m"(*p));
>
> We indeed have explicit "lwz" and "stw" instructions in there.
>
>>
>> where that last line should actually read
>>
>> asm ("lwz%X1 %0,%1" : "=r"(x) : "m"(*p));
>
> Indeed, turning those into "lwzx" and "stwx" seems to fix the issue.
>
> There has been some level of extra CPP macro coating around those instructions
> to
> support both ppc32 and ppc64 with the same assembly. So adding %X[arg] is not
> trivial.
> Let me see what can be done here.
I did the following changes which appear to generate valid asm.
See attached corresponding .S output.
I grepped for uses of "m" asm operand in Linux powerpc code and noticed it's pretty much
always used with e.g. "lwz%U1%X1". I could find one blog post discussing that %U is about
update flag, and nothing about %X. Are those documented ?
Although it appears to generate valid asm, I have the feeling I'm relying on undocumented
features here. :-/
Here is the diff on https://git.kernel.org/pub/scm/libs/librseq/librseq.git/tree/include/rseq/rseq-ppc.h
It's only compile-tested on powerpc32 so far:
diff --git a/include/rseq/rseq-ppc.h b/include/rseq/rseq-ppc.h
index eb53953..f689fe9 100644
--- a/include/rseq/rseq-ppc.h
+++ b/include/rseq/rseq-ppc.h
@@ -47,9 +47,9 @@ do { \
#ifdef __PPC64__
-#define STORE_WORD "std "
-#define LOAD_WORD "ld "
-#define LOADX_WORD "ldx "
+#define STORE_WORD(arg) "std%U[" __rseq_str(arg) "]%X[" __rseq_str(arg) "] " /* To memory ("m" constraint) */
+#define LOAD_WORD(arg) "lwd%U[" __rseq_str(arg) "]%X[" __rseq_str(arg) "] " /* From memory ("m" constraint) */
+#define LOADX_WORD "ldx " /* From base register ("b" constraint) */
#define CMP_WORD "cmpd "
#define __RSEQ_ASM_DEFINE_TABLE(label, version, flags, \
@@ -89,9 +89,9 @@ do { \
#else /* #ifdef __PPC64__ */
-#define STORE_WORD "stw "
-#define LOAD_WORD "lwz "
-#define LOADX_WORD "lwzx "
+#define STORE_WORD(arg) "stw%U[" __rseq_str(arg) "]%X[" __rseq_str(arg) "] " /* To memory ("m" constraint) */
+#define LOAD_WORD(arg) "lwz%U[" __rseq_str(arg) "]%X[" __rseq_str(arg) "] " /* From memory ("m" constraint) */
+#define LOADX_WORD "lwzx " /* From base register ("b" constraint) */
#define CMP_WORD "cmpw "
#define __RSEQ_ASM_DEFINE_TABLE(label, version, flags, \
@@ -125,7 +125,7 @@ do { \
RSEQ_INJECT_ASM(1) \
"lis %%r17, (" __rseq_str(cs_label) ")@ha\n\t" \
"addi %%r17, %%r17, (" __rseq_str(cs_label) ")@l\n\t" \
- "stw %%r17, %[" __rseq_str(rseq_cs) "]\n\t" \
+ "stw%U[" __rseq_str(rseq_cs) "]%X[" __rseq_str(rseq_cs) "] %%r17, %[" __rseq_str(rseq_cs) "]\n\t" \
__rseq_str(label) ":\n\t"
#endif /* #ifdef __PPC64__ */
@@ -136,7 +136,7 @@ do { \
#define RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, label) \
RSEQ_INJECT_ASM(2) \
- "lwz %%r17, %[" __rseq_str(current_cpu_id) "]\n\t" \
+ "lwz%U[" __rseq_str(current_cpu_id) "]%X[" __rseq_str(current_cpu_id) "] %%r17, %[" __rseq_str(current_cpu_id) "]\n\t" \
"cmpw cr7, %[" __rseq_str(cpu_id) "], %%r17\n\t" \
"bne- cr7, " __rseq_str(label) "\n\t"
@@ -153,25 +153,25 @@ do { \
* RSEQ_ASM_OP_* (else): doesn't have hard-code registers(unless cr7)
*/
#define RSEQ_ASM_OP_CMPEQ(var, expect, label) \
- LOAD_WORD "%%r17, %[" __rseq_str(var) "]\n\t" \
+ LOAD_WORD(var) "%%r17, %[" __rseq_str(var) "]\n\t" \
CMP_WORD "cr7, %%r17, %[" __rseq_str(expect) "]\n\t" \
"bne- cr7, " __rseq_str(label) "\n\t"
#define RSEQ_ASM_OP_CMPNE(var, expectnot, label) \
- LOAD_WORD "%%r17, %[" __rseq_str(var) "]\n\t" \
+ LOAD_WORD(var) "%%r17, %[" __rseq_str(var) "]\n\t" \
CMP_WORD "cr7, %%r17, %[" __rseq_str(expectnot) "]\n\t" \
"beq- cr7, " __rseq_str(label) "\n\t"
#define RSEQ_ASM_OP_STORE(value, var) \
- STORE_WORD "%[" __rseq_str(value) "], %[" __rseq_str(var) "]\n\t"
+ STORE_WORD(var) "%[" __rseq_str(value) "], %[" __rseq_str(var) "]\n\t"
/* Load @var to r17 */
#define RSEQ_ASM_OP_R_LOAD(var) \
- LOAD_WORD "%%r17, %[" __rseq_str(var) "]\n\t"
+ LOAD_WORD(var) "%%r17, %[" __rseq_str(var) "]\n\t"
/* Store r17 to @var */
#define RSEQ_ASM_OP_R_STORE(var) \
- STORE_WORD "%%r17, %[" __rseq_str(var) "]\n\t"
+ STORE_WORD(var) "%%r17, %[" __rseq_str(var) "]\n\t"
/* Add @count to r17 */
#define RSEQ_ASM_OP_R_ADD(count) \
@@ -196,11 +196,11 @@ do { \
"333:\n\t" \
#define RSEQ_ASM_OP_R_FINAL_STORE(var, post_commit_label) \
- STORE_WORD "%%r17, %[" __rseq_str(var) "]\n\t" \
+ STORE_WORD(var) "%%r17, %[" __rseq_str(var) "]\n\t" \
__rseq_str(post_commit_label) ":\n\t"
#define RSEQ_ASM_OP_FINAL_STORE(value, var, post_commit_label) \
- STORE_WORD "%[" __rseq_str(value) "], %[" __rseq_str(var) "]\n\t" \
+ STORE_WORD(var) "%[" __rseq_str(value) "], %[" __rseq_str(var) "]\n\t" \
__rseq_str(post_commit_label) ":\n\t"
static inline __attribute__((always_inline))
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: basic_percpu_ops_test-fix.S
Type: application/octet-stream
Size: 91499 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20200708/549a4b06/attachment-0001.obj>
More information about the Linuxppc-dev
mailing list