Inline Assembly: Memory constraints with offsets

Hi,

I’m trying to implement the ZC inline assembly constraint for Mips. This constraint is a memory constraint that expands to an address with an offset (the range of the offset varies according to the subtarget), so the inline assembly in:

int data[10];

void ZC(void) {

asm volatile (“foo %0 %1” : : “ZC”(data[1]), “ZC”(data[2]));

}

Should expand to something like:

foo 4($2) 8($2)

At the moment, the best I can get is something like:

foo 0($2) 0($3)

with the offsets being added before the inline assembly.

Does anyone have any suggestions as to how I can get the offset inside the inline assembly?

Thanks

Daniel Sanders

Leading Software Design Engineer, MIPS Processor IP

Imagination Technologies Limited

www.imgtec.com

How are you actually getting this to compile? I just built clang and I'm getting an error:

clang-3.6 -target mips -S mipsa.c
mipsa.c:4:33: error: invalid input constraint 'ZC' in asm
   asm volatile ("foo %0 %1" : : "ZC"(data[1]), "ZC"(data[2]));
                                 ^
1 error generated.

It doesn't seem that the function "validateAsmConstraint" in tools/clang/lib/Basic/Targets.cpp handles "ZC" as a constraint.

-Krzysztof

Does anyone have any suggestions as to how I can get the offset inside the
inline assembly?

I think x86 has the same capability with a simple "m" constraint.
Looks like it's based on the "SelectInlineAsmMemoryOperand" function.

Cheers.

Tim.

Thanks. I came across SelectInlineAsmMemoryOperand but the caller I found is passing a hardcoded 'm' into it. I'll start by seeing if the 'o' and 'v' paths really trigger for X86.

From: llvmdev-bounces@cs.uiuc.edu [mailto:llvmdev-bounces@cs.uiuc.edu]
On Behalf Of Krzysztof Parzyszek
Sent: 03 March 2015 14:35
To: llvmdev@cs.uiuc.edu
Subject: Re: [LLVMdev] Inline Assembly: Memory constraints with offsets

> Hi,
>
> I'm trying to implement the ZC inline assembly constraint for Mips. This
> constraint is a memory constraint that expands to an address with an
> offset (the range of the offset varies according to the subtarget), so
> the inline assembly in:
>
> int data[10];
>
> void ZC(void) {
>
> asm volatile ("foo %0 %1" : : "ZC"(data[1]), "ZC"(data[2]));
>
> }
>
> Should expand to something like:
>
> foo 4($2) 8($2)
>
> At the moment, the best I can get is something like:
>
> foo 0($2) 0($3)
>
> with the offsets being added before the inline assembly.
>
> Does anyone have any suggestions as to how I can get the offset inside
> the inline assembly?

How are you actually getting this to compile? I just built clang and
I'm getting an error:

clang-3.6 -target mips -S mipsa.c
mipsa.c:4:33: error: invalid input constraint 'ZC' in asm
   asm volatile ("foo %0 %1" : : "ZC"(data[1]), "ZC"(data[2]));
                                 ^
1 error generated.

It doesn't seem that the function "validateAsmConstraint" in
tools/clang/lib/Basic/Targets.cpp handles "ZC" as a constraint.

-Krzysztof

Partial support for ZC is in my working copy at the moment. I've attached my WIP patches.

clang-implement-ZC.patch (1.15 KB)

llvm-implement-ZC.patch (975 Bytes)

It turns out that those paths don't trigger. I've made a small change to print the ConstraintCode when this function is called and found that the following code:
  int data[10];
  void o(void) {
    asm volatile ("foo %0" : : "o"(data[1]));
  }
  // The 'v' case is commented out because clang doesn't actually accept it in the frontend despite having a backend implementation.
  //void v(void) {
  // asm volatile ("foo %0" : : "v"(data[1]));
  //}
  void m(void) {
    asm volatile ("foo %0" : : "m"(data[1]));
  }
when compiled for X86, only prints 'm' during compilation and never 'o'. It appears that only 'm' and equivalent constraints can be implemented.

I think the 'm' is hardcoded because the constraints aren't present in the SelectionDAG nodes and I think they're not present because it's difficult to represent them. I expect the 'm' was good enough at the time and Mips might be the first target to need sufficiently complicated constraints for it to be a problem. I'll have to have a think as to how I can deliver the constraint code to this function nicely.

Should have guessed that, ha.

I've looked into this. My idea was to expand the single address operand of the inline-asm SDNode into two: base pointer and offset.

I haven't found a place where all needed information would be readily available, and where the common code would do all the work based on some target-specific hook. Failing that, my last resort approach would be to have a pre-isel pass that expands the inline-asm, potentially changing the constraints to reflect the connection between the register and the constant input operands (needed for the asm-printer).

Not sure if this is of any help.

-Krzysztof

From: Krzysztof Parzyszek [kparzysz@codeaurora.org]
>
> Partial support for ZC is in my working copy at the moment. I've attached my WIP patches.

Should have guessed that, ha.

I've looked into this. My idea was to expand the single address operand
of the inline-asm SDNode into two: base pointer and offset.

I haven't found a place where all needed information would be readily
available, and where the common code would do all the work based on some
target-specific hook. Failing that, my last resort approach would be to
have a pre-isel pass that expands the inline-asm, potentially changing
the constraints to reflect the connection between the register and the
constant input operands (needed for the asm-printer).

Not sure if this is of any help.

-Krzysztof

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

I've had some luck with this. It turns out that the ISD::INLINE_ASM node has a flag operand corresponding to each operand in the source code. It's used for storing things like the matched operand number for the [0-9]+ constraints, the register class id for register constraints, etc. At first glance, the encoding looks full but it appears that when the lowest 3 bits are Kind_Mem (meaning it's a memory constraint), the upper 16-bits are never used. I've therefore used this gap to store a constraint ID which can then be inspected during instruction selection. The mapping from constraint string to constraint id is provided by a target specific hook with unmapped constraints causing a failure (currently an assertion but it should be llvm_unreachable). I now have the Mips 'ZC' constraint supporting 12-bit offsets while 'm' continues to lack support for offsets. This isn't the correct behaviour for 'ZC' or for 'm' but it does at least demonstrate that different memory constraints can now behave differently. My WIP patch is at http://reviews.llvm.org/D8160. It's going to be split up into multiple patches and the Mips portion is going to be corrected before I upstream it. I'll also check the clang tests to see if I've missed any supported constraints.

There's a few odd target specific things in the current code so I've CC'd the code owners. I couldn't find a code owner for PowerPC or MSP430 in CODE_OWNERS.txt though.

The Hexagon and X86 backends both had code that attempted to implement different behaviour for the 'o' and 'v' memory constraints but this code has been unused so far because the backend always received the code 'm'. I attempted to use the existing implementations for these but this causes test failures so I've left them behaving like 'm'.

The AArch64, ARM, PowerPC, and SystemZ backends have tests for a few constraints that currently behave like 'm'. I assume that keeping the 'm' behaviour is ok for now.

There's also a change to the MSP430 backend that should be non-functional but I ought to mention. The change is that the inline assembly flags are now i32 instead of a pointer-sized int. This is because it will attempt to store a 32-bit value in an i16 otherwise.

node has a flag operand corresponding to each operand in the source
code. It's used for storing things like the matched operand number
for the [0-9]+ constraints, the register class id for register
constraints, etc. At first glance, the encoding looks full but it
appears that when the lowest 3 bits are Kind_Mem (meaning it's a
memory constraint), the upper 16-bits are never used. I've therefore
used this gap to store a constraint ID which can then be inspected
during instruction selection. The mapping from constraint string to
constraint id is provided by a target specific hook with unmapped
constraints causing a failure (currently an assertion but it should
be llvm_unreachable). I now have the Mips 'ZC' constraint supporting
12-bit offsets while 'm' continues to lack support for offsets. This
isn't the correct behaviour for 'ZC' or for 'm' but it does at least
demonstrate that different memory constraints can now behave
differently. My WIP patch is at http://reviews.llvm.org/D8160. It's
going to be split up into multiple patches and the Mips portion is
going to be corrected before I upstream it. I'll also check the
clang tests to see if I've missed any supported constraints.

There's a few odd target specific things in the current code so I've
CC'd the code owners. I couldn't find a code owner for PowerPC or
MSP430 in CODE_OWNERS.txt though.

The code owner for PowerPC is Hal Finkel (added on CC).

The Hexagon and X86 backends both had code that attempted to
implement different behaviour for the 'o' and 'v' memory constraints
but this code has been unused so far because the backend always
received the code 'm'. I attempted to use the existing
implementations for these but this causes test failures so I've left
them behaving like 'm'.

The AArch64, ARM, PowerPC, and SystemZ backends have tests for a few
constraints that currently behave like 'm'. I assume that keeping
the 'm' behaviour is ok for now.

Sure. Once the infrastructure is in, I'll update SystemZ to make use
of the additional constraints. Thanks for taking care of this!

Bye,
Ulrich

From: "Ulrich Weigand" <Ulrich.Weigand@de.ibm.com>
To: "Daniel Sanders" <Daniel.Sanders@imgtec.com>
Cc: adasgupt@codeaurora.org, "evan cheng" <evan.cheng@apple.com>, "Krzysztof Parzyszek" <kparzysz@codeaurora.org>,
llvmdev@cs.uiuc.edu, nrotem@apple.com, "t p northover" <t.p.northover@gmail.com>, "Hal Finkel" <hfinkel@anl.gov>
Sent: Monday, March 9, 2015 9:30:26 AM
Subject: RE: [LLVMdev] Inline Assembly: Memory constraints with offsets

> I've had some luck with this. It turns out that the ISD::INLINE_ASM
> node has a flag operand corresponding to each operand in the source
> code. It's used for storing things like the matched operand number
> for the [0-9]+ constraints, the register class id for register
> constraints, etc. At first glance, the encoding looks full but it
> appears that when the lowest 3 bits are Kind_Mem (meaning it's a
> memory constraint), the upper 16-bits are never used. I've
> therefore
> used this gap to store a constraint ID which can then be inspected
> during instruction selection. The mapping from constraint string to
> constraint id is provided by a target specific hook with unmapped
> constraints causing a failure (currently an assertion but it should
> be llvm_unreachable). I now have the Mips 'ZC' constraint
> supporting
> 12-bit offsets while 'm' continues to lack support for offsets.
> This
> isn't the correct behaviour for 'ZC' or for 'm' but it does at
> least
> demonstrate that different memory constraints can now behave
> differently. My WIP patch is at http://reviews.llvm.org/D8160. It's
> going to be split up into multiple patches and the Mips portion is
> going to be corrected before I upstream it. I'll also check the
> clang tests to see if I've missed any supported constraints.
>
> There's a few odd target specific things in the current code so
> I've
> CC'd the code owners. I couldn't find a code owner for PowerPC or
> MSP430 in CODE_OWNERS.txt though.

The code owner for PowerPC is Hal Finkel (added on CC).

I'd like to second Uli's comment: Thanks for working on this! This is a code quality issue that affects PowerPC too.

-Hal

Fantastic! Yes, this is one of the places that I looked at, but to my great disappointment the connection with the original constraint was not preserved.

Thanks for doing this!

-Krzysztof