Codegen constraint improvements

[llvmbot]

Bugzilla Link	2117
Resolution	FIXED
Resolved on	Oct 15, 2008 11:59
Version	trunk
OS	Linux
Reporter	LLVM Bugzilla Contributor
CC	@asl,@isanbard

Extended Description

Since a number of operations produce results in certain registers or take input in certain registers, it would be very nice to be able to say:

let Constraints = "$dst = %eax, $val = %ecx" in
let Constraints = "$dst = %eax:%edx" in

etc

and have the selection dag insert the to and from reg nodes automatically. This seems like it would cut down on a lot of custom lowering (dag op lowered to copyto, op, copyfrom).

[lattner]

I'm pretty sure this already works, it just doesn't use constraints. For example, x86 variable shl is defined as:

def SHL8rCL : I<0xD2, MRM4r, (outs GR8 :$dst), (ins GR8 :$src),
"shl{b}\t{%cl, $dst|$dst, %CL}",
[(set GR8:$dst, (shl GR8:$src, CL))]>;

and the isel automatically copies the shift amount into CL.

-Chris

[llvmbot]

Maybe it is something to do with the register classes on x86, but I tried doing that for the LCMPXCHG patterns (since I saw it down in the muls) and I could not get the code through. It would parse, and it seemed like it would emit copyto copyfrom, but those nodes couldn't find the register class the register was in. I also notice that the x86 backend only does this for 8 bit operations, the similar register tied 16 bit operations are not pattern matched that way.

[lattner]

Can you include the pattern that you tried?

[asl]

Can you include the pattern that you tried?

This is x86 atomic.las pattern, which should be turned into xadd instruction. This is pretty "bad" instruction, because it's encoded as "MRMDestMem", but also produce a value in register, so, in some sence it's both "rm" and "mr" instruction. Which makes JIT emitter to fail :(

[lattner]

xadd looks like a standard 2-address instruction to me, it doesn't require a value in any specific register.

[lattner]

cmpxchg seems like it has implicit def/uses, in addition to being mod/rm. How did you try to model it?

[llvmbot]

What I was trying was:

let Defs = [EAX, EFLAGS], Uses = [EAX] in {
def LCMPXCHG32 : I<0xB1, Pseudo, (outs), (ins i32mem:$ptr, GR32:$swap),
"lock cmpxchgl $swap,$ptr",
[(set EAX (atomic_lcs_32 addr:$ptr, EAX, GR32:$swap))]>, TB, LOCK;
}

which looked from the MUL8r example like it should work. But no variation of that pattern (specifying type sizes like i32 at places, etc) would codegen. They would make it through tablegen, but if you fed it an llvm.atomic.lcs.i32 it would die on one of the copyin copyout nodes it seemed to generate claiming it couldn't find a register class that contained EAX. I think this is because it was trying to find a register class for type MVT::Other that contained EAX. At least this was as much as I deduced in gdb before giving up and writing the lowering code.

I tried variations of that that didn't include the Defs or Uses line too.

LCMPXCHG32 was having type ch,ch (no outs obviously), which is why it was looking up EAX for MVT::Other.

What would be really nice to write is:

let Constraints = "$cmp = EAX, $dst = EAX", DontEncode = "$cmp, $dst" in {
def LCMPXCHG32 : I<0xB1, Pseudo, (outs $dst), (ins i32mem:$ptr, GR32:$cmp, GR32:$swap),
"lock cmpxchgl $swap,$ptr",
[(set GR32:$dst (atomic_lcs_32 addr:$ptr, GR32:$cmp, GR32:$swap))]>, TB, LOCK;
}

if I could write:

def LCMPXCHG32 : I<0xB1, Pseudo, (outs), (ins i32mem:$ptr, GR32:$swap),
"lock cmpxchgl $swap,$ptr",
[(set EAX (atomic_lcs_32 addr:$ptr, EAX, GR32:$swap))]>, TB, LOCK;

or

def LCMPXCHG32 : I<0xB1, Pseudo, (outs EAX), (ins i32mem:$ptr, EAX, GR32:$swap),
"lock cmpxchgl $swap,$ptr",
[(set EAX (atomic_lcs_32 addr:$ptr, EAX, GR32:$swap))]>, TB, LOCK;

and have it work, that would be even better.

right now the lowering for the instruction is to a copyto, lcmpxchgl, copyfrom with flag operands and results, it just seems like it should be automated, and almost is.

[llvmbot]

Not sure why your pattern doesn't work. It seems alright to me. How is atomic_lcs_32 defined?

[lattner]

I assume this works, we support atomic ops now, and use physreg constraints all over the place.