[llvm-commits] [PATCH] MC Assembler does not support DSB correctly

[Jim Grosbach]

Hi Richard,

LGTM.

-Jim
On Jun 21, 2012, at 4:10 AM, Richard Barton <richard.barton at arm.com> wrote:

> Hello Reviewers
> 
> The ARM MC Assembler does not support DSB instructions correctly.
> 
> The DSB (Data Synchronization Barrier) instruction ensures the completion of
> memory accesses and has assembly syntax:
> 
> DSB{<c>}{<q>}  {<option>}
> 
> Where the optional <option> value can impose a limitation on the types of
> memory accesses effected. There are a number of proscribed assembly strings for
> this option, the default is "SY"
> 
> The ARMARM says the following about these values:
> 
> "All other encodings of option are reserved. It is implementation defined
> whether options other than SY are implemented. All unsupported and reserved
> options must execute as a full system DSB operation, but software must not rely
> on this behavior."
> 
> Currently, the MC Assembler does not have any syntax for the reserved option
> values. GNU as recognises "DSB 0x0" for example as the reserved option value 0.
> The ARM Assembler throws an assertion failure in this situation.
> 
> The assertion failure causes problems for our MC Hammer testing. The attached
> patch 
> (dsb_assembly_no_assertion.patch) converts this assertion failure into an error
> message. 
> I have not added a regression test for this fix as I think that the error
> message is 
> still not the correct outcome in this instance (see bugzilla link), and I don't
> want to 
> bake that behaviour into the tests. That said, the assembler should not be
> internal
> faulting a syntax error, so the error message is the correct solution until the
> full 
> fix is ready.
> 
> There is an additional problem in that the assembler does not recognise an
> uppercase 
> string for the option field. The attached patch (dsb_capital_operation.patch)
> fixes this issue
> and adds a regression test.
> 
> The remaining issues that I have discovered with this instruction are lower on
> our 
> priority list so I don't have a fix for these now. 
> 
> I have raised 13138 below to record these issues so a fix can be made in future:
> 
> http://llvm.org/bugs/show_bug.cgi?id=13138
> 
> Please review,
> 
> Regards,
> Richard Barton
> ARM Ltd, Cambridge
> 
> <dsb_assembly_no_assertion.patch><dsb_capital_operation.patch>