[PATCH] Set "thumbv6k" CPU subtype to ARM_V6

Renato Golin via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 12 10:22:01 PDT 2015


On 12 August 2015 at 17:06, Vedant Kumar <vsk at apple.com> wrote:
> It looks like MachO is the only file format that cares about the ARM CPU Subtype.

It is.


> By vague, do you mean that the patch should not be MachO-specific? There don't appear to be any issues with compiling with -mtriple=thumbv6k-windows-gnu or -mtriple=thumbv6k-linux-gnu..

By vague I mean you haven't looked at the bigger picture.

We used to fix these kind of problems by adding more and more
StringSwitch matches until it became unmaintainable.

Now, we moved to using the ARMTargetParser, which unifies both the
Clang driver and the assembler string parsing routines, so that both
identify and report consistent behaviour.

Right now, the ARMTargetParser is hand-coded and will remain so until
we're sure that all needs are covered, when we'll create a new
table-gen back-end to generate it from the *.td files in each target,
and make that generation compulsory even if the target is not built
with LLVM or Clang.


> Hm. This would require a clang driver change.

You seem to be doing that already. Even though you're changing the
assembler, this has repercussions to the Clang driver.

Tim, how is the triple done in Darwin? Do you guys use thumbv7 or
prefer armv7+mthumb? Would it be better to convert thumbv7
->armv7+mthumb, or leave it as it is?

The ARMTargetParser understands both, but it spits our armv*, so you'd
have to keep the original string in order to pass it along down to the
assembler, if that's what you need.

Check ARMTargetParser::getSubArch();


> Would it be better if I sent a patch for this to cfe-commits?

The best thing to do now would be to send a patch to Phabricator,
copying llvm-commitd and cfe-commits, plus the people replying to you
in this thread, to start.


> Could you clarify what's missing in the patch?

Apart from the bigger picture, as I outlined above, the one you sent
tells me that there isn't any other of such tests in the test suite,
which I find hard to believe. Maybe that's true, but I'd rather be
certain.

Regardless, you should add a few more arch-name tests to make sure
that other oddities don't show up.

cheers,
--renato


More information about the llvm-commits mailing list