Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Input constraints for inline assembler are not used for CSE #9867

Open Quuxplusone opened 13 years ago

Quuxplusone commented 13 years ago
Bugzilla Link PR9517
Status NEW
Importance P normal
Reported by Jörg Sonnenberger (joerg@NetBSD.org)
Reported on 2011-03-20 16:41:11 -0700
Last modified on 2018-07-23 13:23:22 -0700
Version trunk
Hardware PC Linux
CC echristo@gmail.com, efriedma@quicinc.com, llvm-bugs@lists.llvm.org, llvm-dev@ndave.org
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
Consider the following program:

static inline void
outb(unsigned short port, unsigned char data)
{
        __asm volatile("outb %%al,%w1" : : "a" (data), "d" (port));
}

extern unsigned short base;

void test(void)
{
        outb(base + 16, 0x0);
        outb(base + 16, 0x1);
        outb(base + 16, 0x2);
        outb(base + 16, 0x3);
        outb(base + 16, 0x4);
        outb(base + 16, 0x5);
        outb(base + 16, 0x6);
        outb(base + 16, 0x7);
        outb(base + 16, 0x8);
}

This should be compiled into something like:

movzwl base, %edx
add 0x10, %edx
xorl %eax, %eax
outb %al,%dx
movb $1, %al
outb %al,%dx
movb $2, %al
outb %al,%dx
...

or even better in terms of code size:

movzwl base, %edx
add 0x10, %edx
xorl %eax, %eax
outb %al,%dx
incl %eax
outb %al,%dx
incl %eax
outb %al,%dx
...

But currently it gets compiled to:

movzwl base, %edx
add 0x10, %edx
xorl %eax, %eax
outb %al,%dx
movzwl base, %edx
add 0x10, %edx
movb $1, %al
outb %al,%dx
movzwl base, %edx
add 0x10, %edx
movb $2, %al
outb %al,%dx
...

Input constraints are not clobbered and therefore, they don't have to be
reloaded. The reload overhead in this case is significant.
Quuxplusone commented 13 years ago

The issue here is actually that the loads are not being CSE'ed; alias analysis is being overly conservative, in that it assumes the inline asm can modify "base".

Quuxplusone commented 13 years ago
(In reply to comment #1)
> The issue here is actually that the loads are not being CSE'ed; alias analysis
> is being overly conservative, in that it assumes the inline asm can modify
> "base".

I wasn't sure about that. It shouldn't assume that since there is no clobber
"memory" specified.
Quuxplusone commented 13 years ago

It shouldn't, but it does. There doesn't happen to be any convenient way to represent that the inline asm has side effects, but doesn't modify any memory visible to the compiler.

Quuxplusone commented 13 years ago

The volatile already expresses "this has side effects".

Quuxplusone commented 10 years ago

Still applies as of r200573.

Quuxplusone commented 6 years ago

https://reviews.llvm.org/D49691