[PATCH] D19061: [ARM] Add support for the X asm constraint

[Renato Golin via llvm-commits]

rengolin added a comment.

In http://reviews.llvm.org/D19061#410492, @jgreenhalgh wrote:

> If you're happy to be more restrictive than GCC here, then I think think the patch is fine.


We're always happy to be more restrictive than GCC. :)

> Please heavily comment the testcases to reflect that they are not representative of the way the "X" constraint should be used in the real world, and you may want to add further comments to the lowering function to make it clear that it is tighter than neccessary.


Indeed, please do! Also, if you could add the asm() statement that generated each example and a short explanation where the match is binding to, it'd be most helpful, given that you can't match the parameter in some cases.

With the additional comments in the test, LGTM. Thanks!


================
Comment at: test/CodeGen/ARM/inlineasm-X-constraint.ll:4
@@ +3,3 @@
+; CHECK-LABEL: f1
+; CHECK: vmsr fpscr
+; CHECK: vadd.f64
----------------
sbaranga wrote:
> rengolin wrote:
> > shouldn't you verify that the register used in vmsr is also used in vadd?
> > 
> > Same comment for other CHECK lines below.
> The vadd operand ($0) doesn't appear in the vmsr instruction. It's there just to add the dependency (which is the usecase shown in the PR).
Right, ok.


http://reviews.llvm.org/D19061