[PATCH] Use ".arch_extension" ARM directive to specify the additional CPU features
renato.golin at linaro.org
Sun Feb 8 12:56:27 PST 2015
Tim, Eric, any more comments?
I think for what it is, the patch is good to go. The sub-target refactoring is already covered by a bug I own (sub-target description classes).
Comment at: lib/Target/ARM/ARMAsmPrinter.cpp:644
@@ -643,3 +643,1 @@
- // FIXME: remove krait check when GNU tools support krait cpu
- if (CPUString != "generic" && CPUString != "krait")
> rengolin wrote:
> > t.p.northover wrote:
> > > Any news on this, by the way? It's rather a horrific hack to have to carry, and it only seems to be getting worse.
> > You know, I've been thinking... With the number of third-party ARM CPUs out there, maybe it would be a better model to emit .cpu as a core ARM name, and use .arch_extension to all little differences, for all other cores.
> > Of course, "if (Krait)" is not what I'm suggesting, but maybe a spec of each CPU somewhere, possible extracted from TableGen files, could go a long way of doing this right and transparent...
> > to emit .cpu as a core ARM name, and use .arch_extension to all little differences, for all other cores
> This is exactly what I am trying to implement here.
> > possible extracted from TableGen files, could go a long way of doing this right and transparent.
> I second your opinion except that I have no clue on how to implement it. This is my first patch to LLVM :). I need a lot more time to go through most of the components of LLVM
Sorry, I didn't mean this patch should have that refactoring, just that this technique isn't pointing us in the wrong direction. The idea is to ultimately remove all isKrait() and similar checks from the back-end, but this is not possible today.
I agree with Tim that the new code looks it's going in the wrong direction, but in the grand scheme of things, it may not. Plus, that refactoring will have to be done more thoroughly at once, from the table-gen side.
More information about the llvm-commits