Re: [PATCH v2] powerpc: slightly improve cache helpers

On Thu, Jul 25, 2019 at 2:30 PM Nick Desaulniers
[off-list ref] wrote:

On Mon, Jul 22, 2019 at 10:58 AM Segher Boessenkool
[off-list ref] wrote:

quoted

On Mon, Jul 22, 2019 at 10:21:07AM -0700, Nick Desaulniers wrote:

quoted

On Sun, Jul 21, 2019 at 11:19 PM Segher Boessenkool
[off-list ref] wrote:

quoted

On Sun, Jul 21, 2019 at 07:41:40PM -0700, Nathan Chancellor wrote:

quoted

On Sun, Jul 21, 2019 at 01:01:50PM -0500, Segher Boessenkool wrote:

quoted

On Sun, Jul 21, 2019 at 12:58:46AM -0700, Nathan Chancellor wrote:

quoted

0000017c clear_user_page:
     17c: 94 21 ff f0                     stwu 1, -16(1)
     180: 38 80 00 80                     li 4, 128
     184: 38 63 ff e0                     addi 3, 3, -32
     188: 7c 89 03 a6                     mtctr 4
     18c: 38 81 00 0f                     addi 4, 1, 15
     190: 8c c3 00 20                     lbzu 6, 32(3)
     194: 98 c1 00 0f                     stb 6, 15(1)
     198: 7c 00 27 ec                     dcbz 0, 4
     19c: 42 00 ff f4                     bdnz .+65524

Uh, yeah, well, I have no idea what clang tried here, but that won't
work.  It's copying a byte from each target cache line to the stack,
and then does clears the cache line containing that byte on the stack.

I *guess* this is about "Z" and not about "%y", but you'll have to ask
the clang people.

Or it may be that they do not treat inline asm operands as lvalues
properly?  That rings some bells.  Yeah that looks like it.

The code is
  __asm__ __volatile__ ("dcbz %y0" : : "Z"(*(u8 *)addr) : "memory");

so yeah it looks like clang took that  *(u8 *)addr  as rvalue, and
stored that in stack, and then used *that* as memory.

What's the %y modifier supposed to mean here?

It prints a memory address for an indexed operand.

If you write just "%0" it prints addresses that are a single register
as "0(r3)" instead of "0,r3".  Some instructions do not allow offset
form.

quoted

addr is in the list of
inputs, so what's wrong with using it as an rvalue?

It seems to use *(u8 *)addr as rvalue.  Asm operands are lvalues.  It
matters a lot for memory operands.

Hmm...not sure that's specified behavior.  Anyways, I've filed:
https://bugs.llvm.org/show_bug.cgi?id=42762
to see if folks more familiar with LLVM's ppc backend have some more thoughts.

I recommend considering reverting commit 6c5875843b87 ("powerpc:
slightly improve cache helpers") until the issue is resolved in clang,
otherwise I'll probably just turn off our CI builds of PPC32 for the
time being.

Started a new thread: https://lkml.org/lkml/2019/7/29/1483
-- 
Thanks,
~Nick Desaulniers