[dragonegg] cannot tie inline asm operands of different widths

[edwintorok]

Bugzilla Link	3373
Resolution	FIXED
Resolved on	Feb 27, 2011 10:53
Version	trunk
OS	Linux
CC	@sunfishcode

Extended Description

Using yesterday's llvm-gcc (SVN r62706), I fail to build the following:

int foo(unsigned long *addr)
{
int __ret_pu;
unsigned long __pu_val;
return ({asm volatile("call _put_user" "8" : "=a" (__ret_pu) :"0"
(__pu_val), "c"(addr) : "ebx"); __ret_pu;});
}

$ llvm-gcc x.i -O0 -emit-llvm -o x.bc
x.i: In function ‘foo’:
x.i:5: error: unsupported inline asm: input constraint with a matching output constraint of incompatible type!

I thought the frontend was supposed to cast the argument to the proper type to avoid this error message.

LLVM 2.4 accepted this code, and appears to be the kernel people's favourite way to use inline asm (i.e. return type is int, but input is something else).

Can this be fixed for 2.5 please?

[llvmbot]

Also the other way around appears in the kernel.

output is a char, tied input is an int.

[lattner]

We intentionally decided to reject this in 2.5 because the semantics are very vague and difficult to implement. I'd suggest two things:

1. Work with the upstream sources to get them to make their asms more portable.
2. change llvm-gcc to do the "lowering" of this construct (e.g. insert a truncate or extend before/after the asm) in cases where the behavior is well defined.

In either case, I don't think we're going to get this for 2.5, the changes would be too invasive and this is a very touchy area.

[lattner]

We intentionally decided to reject this in 2.5 because the semantics are very vague and difficult to implement. I'd suggest two things:

1. Work with the upstream sources to get them to make their asms more portable.
2. change llvm-gcc to do the "lowering" of this construct (e.g. insert a truncate or extend before/after the asm) in cases where the behavior is well defined.

In either case, I don't think we're going to get this for 2.5, the changes would be too invasive and this is a very touchy area.

[edwintorok]

We intentionally decided to reject this in 2.5 because the semantics are very
vague and difficult to implement. I'd suggest two things:

1. Work with the upstream sources to get them to make their asms more portable.

So "a" output constraint's size must match the "0" input constraint's size, is that the rule?

2. change llvm-gcc to do the "lowering" of this construct (e.g. insert a
   truncate or extend before/after the asm) in cases where the behavior is well
   defined.

Supporting truncate would solve this issue I think, by introducing a temporary of correct size. Is that complicated?

In either case, I don't think we're going to get this for 2.5, the changes
would be too invasive and this is a very touchy area.

I just tried with a 32-bit kernel (so that int and long are same size), and there is one inline asm problems there too:

static struct {
unsigned long address;
unsigned short segment;
} bios32_indirect;
static unsigned long bios32_service(unsigned long service)
{
unsigned char return_code; /* %al /
unsigned long address; / %ebx /
unsigned long length; / %ecx /
unsigned long entry; / %edx */
unsigned long flags;

    local_irq_save(flags);
    __asm__("lcall *(%%edi); cld"
            : "=a" (return_code),
              "=b" (address),
              "=c" (length),
              "=d" (entry)
            : "0" (service),
              "1" (0),
              "D" (&bios32_indirect));

}

I think there are plenty more problems for 64-bit though (put_user for start) :(

[lattner]

So "a" output constraint's size must match the "0" input constraint's size, is
that the rule?

Yep.

Supporting truncate would solve this issue I think, by introducing a temporary
of correct size.

Right.

Is that complicated?

Not insanely complex. However, if the input is smaller than the output, you need an extend on input etc.

[edwintorok]

So "a" output constraint's size must match the "0" input constraint's size, is
that the rule?

Yep.

Supporting truncate would solve this issue I think, by introducing a temporary
of correct size.

Right.

Is that complicated?

Not insanely complex. However, if the input is smaller than the output, you
need an extend on input etc.

I could fix that in the kernel source, by adding a cast, there should be only few cases.
If input is char, output is int, I can cast input to int.

Most of the cases I've seen so far are:
output is int, input is unsigned long
output is char, input is unsigned long.

Does that sound like a fair plan?
Handle truncate in llvm-gcc, fix the extend in the kernel source.

[lattner]

sure!

[llvmbot]

My guess is that this

int __ret_pu;
unsigned long __pu_val;
return ({asm volatile("call _put_user" "8" : "=a" (__ret_pu) :"0"
(__pu_val), "c"(addr) : "ebx"); __ret_pu;});

is simply a mistake in the kernel, a hang-over from the days
when everything was 32 bit. How about the following plan:
try to fix the kernel tree; if the kernel people object and
point out some cases with clear semantics that it would be
sensible to support, implement that. But if the kernel people
are happy to fix this in the kernel, just fix it there.

[edwintorok]

I sent an email to the x86 arch maintainers, asking if they would accept patches regarding this issue: http://lkml.org/lkml/2009/1/23/261

[edwintorok]

Here's another case:
typedef unsigned char u8;
typedef struct {
};
struct operand {
enum {
OP_REG, OP_MEM, OP_IMM, OP_NONE }
type;
unsigned int bytes;
unsigned long val, orig_val, *ptr;
};
struct decode_cache {
u8 b;
struct operand src;
struct operand src2;
struct operand dst;
};
struct x86_emulate_ctxt {
unsigned long eflags;
struct decode_cache decode;
};
struct kvm_vcpu_arch {
struct {
}
rmode;
};
int x86_emulate_insn(struct x86_emulate_ctxt *ctxt, struct x86_emulate_ops *ops) {
struct decode_cache *c = &ctxt->decode;
int rc = 0;
done: if (rc == 1) {
}
twobyte_insn: switch (c->b) {
case 0xac: case 0xad: do {
switch ((c->dst).bytes) {
case 2: do {
unsigned long _tmp;
unsigned short _clv = (c->src2).val;
unsigned short _srcv = (c->src).val;
unsigned short _dstv = (c->dst).val;
asm volatile ( "movl %""0"",%""k" "2""; " "push %""2""; " "push %""2""; " "movl %""5"",%""k" "2""; " "andl %""k" "2"",(""%%rsp""); " "pushf; " "notl %""k" "2""; " "andl %""k" "2"",(""%%rsp""); " "andl %""k" "2"",""64/4""(""%%rsp""); " "pop %""2""; " "orl %""k" "2"",(""%%rsp""); " "popf; " "pop %""0""; " "shrd" "w" " %4,%1 \n" "pushf; " "pop %""2""; " "andl %""5"",%""k" "2""; " "orl %""k" "2"",%""0""; " : "=m" (ctxt->eflags), "+r" (_dstv), "=&r" (_tmp) : "c" (_clv) , "r" (_srcv), "i" (((1<<11)|(1<<7)|(1<<6)|(1<<4)|(1<<2)|(1<<0))) );
}
while (0);
}
}
while (0);
}
}

assembly generated by gcc:
#APP

39 "testcase-min.i" 1

    movl (%rsi),%ebx; push %rbx; push %rbx; movl $2261,%ebx; andl %ebx,(%rsp); pushf; notl %ebx; andl %ebx,(%rsp); andl %ebx,64/4(%rsp); pop  %rbx; orl  %ebx,(%rsp); popf; pop  (%rsi); shrdw %dx,%ax

pushf; pop %rbx; andl $2261,%ebx; orl %ebx,(%rsi);

0 "" 2

#NO_APP

[llvmbot]

Here is another example from libatomic-ops / Boehm GC where an automatic truncate should help:

AO_TS_VAL_t nop_full() {
unsigned char oldval;
AO_TS_t dummy;
AO_TS_t *addr = &dummy;
asm volatile("xchgb %0, %1"
: "=q"(oldval), "=m"(*addr)
: "0"(0xff), "m"(*addr) : "memory");
// does work: : "0"((unsigned char)0xff), "m"(*addr) : "memory");
return (AO_TS_VAL_t)oldval;
}

[llvmbot]

Bug #​4077 might be a duplicate.

[llvmbot]

Latest clang & LLVM svn update gives much better diagnostics on this problem.

[edwintorok]

patch to Linux kernel to workaround unsupported inline asm constraints in LLVM

[edwintorok]

Created an attachment (id=2901) [details]
patch to Linux kernel to workaround unsupported inline asm constraints in LLVM

Patch added on Jeff Garzik's request.
Its the same (modulo whitespace differences) as the one I already attached to #​4068:
http://llvm.org/bugs/attachment.cgi?id=2896
http://llvm.org/bugs/attachment.cgi?id=2880