[PATCH] Allow encoded 8-bit floating point constants in ARM vmov instructions

David Peixotto dpeixott at codeaurora.org
Tue Dec 17 12:15:40 PST 2013


> >> Why do we want fconstf/fconstd aliases? Those are pre-unified syntax
> >> instructions, which we're trying to remove from LLVM, not add.
> >
> > Mainly because I see these instructions still appearing in code and
> > gcc accepts them.  We already support a number of aliases for
> > pre-unified syntax (e.g. flds, fldd). I count at least 40 in
> lib/Target/ARM/ARMInstrVFP.td.
> 
> All of which I would very much like to remove. In retrospect, it was an
> error to add the ones we (I) did. It was convenient at the time, but the
> wrong precedent to set.
> 
> > Is there a reason not to support them? I think they maps fairly
> > cleanly to the vmov.f32/vmov.f64 instructions.
> 
> It's explicitly adding support for an assembly syntax that has been
> obsoleted by ARM for over 6 years now and which ARM has indicated they
> have no interest in supporting. Adding more support for divided syntax
> increases the maintenance cost of LLVM's ARM assembly parser, which is
> already a bit overcomplicated. Projects still using divided syntax should
> be fixed to use unified syntax instead.

The arm and gcc assmblers both accept the fconstd alias, even in the context
of unified syntax files. While I agree with you that projects should update
their source code I don't always have the leverage to make that happen :)

Perhaps we can put the issue of adding an fconstd alias aside for a moment
and just address the specific fix in this patch. The current code for
parsing 8-bit floating point constants is broken in two ways:

1. It converts the literal to a double, but the isFPImm() code expects to
find a float so it is not recognized as an fp immediate by the asm matcher.

2. It uses the wrong APFloat api to create the immediate which causes us to
drop the non-integer part of the floating point constant.

The patch I submitted fixes these two issues. Currently, we get a parse
error on this code

  vmov.f32 s0, #0x70

but it looks like that is quite by accident (due to issue 1 above). Do we
want to support writing these constants or should we only allow
floating-point literals (e.g. #1.0 instead of #0x70)?












More information about the llvm-commits mailing list