----- On Jul 8, 2020, at 8:33 AM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote: > ----- On Jul 7, 2020, at 8:59 PM, Segher Boessenkool segher@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