[llvm-commits] [PATCH] ULEB FIXME in ARM back-end

[Renato Golin]

Ping?

On 1 August 2011 13:20, Renato Golin <renato.golin at arm.com> wrote:
> Hi all,
>
> I've been playing with a FIXME in the ARM back-end (to get more
> acquainted) and one simple FIXME was an ULEB printing in the
> AsmPrinter.
>
> According to the ARM EABI, both build attribute tag and value are
> ULEBs and not chars as they were being emitted. Today, I don't know of
> any attribute that would emit tags or values bigger than 127 but this
> seamed like an easy start.
>
> The patch changes the emission of values to ULEB by storing all tags
> and values in a typed list and emit later the right value, computing
> the size as it goes. I've run all tests (including the build
> attributes ones in ARM be) and it passes everything.
>
> There are three problems I see with this patch, but would like to know
> from you what's the best way to solve them:
>
>  1. It uses a local structure to hold both ULEB and String values in
> order, but other MC structures will also emit ULEB in the near future
> (we should make sure it does, at least), so this structure could
> become a proper small class on a more visible place.
>
>  2. It wastes memory since both number and string values are on every
> item, but it's not possible to put StringRef in an union since
> Stringref has non-trivial constructors. A solution is to use
> polymorphism but that's too big for this tiny case. If we decide to
> expose the class higher up, then it makes sense to do so.
>
>  3. It's calculating the ULEB size in place, where other routines in
> LLVM can do that (but are not accessible from this class). A
> refactoring is in need to expose that routine to the intended
> audience.
>
> If neither of those problems are relevant, or if they're just minor,
> we can put other FIXMEs around. Otherwise, I'm open to suggestions.
>
> best,
> --renato
>



-- 
cheers,
--renato