Page MenuHomePhabricator

Handle ARMv6-J as an alias, instead of fake architecture
ClosedPublic

Authored by tyomitch on Nov 17 2015, 2:22 PM.

Details

Summary

This follows D14577 to treat ARMv6-J as an alias for ARMv6,
instead of an architecture in its own right.

The functional change is that the default CPU when targeting ARMv6-J
changes from arm1136j-s to arm1136jf-s, which is currently used as
the default CPU for ARMv6; both are, in fact, ARMv6-J CPUs.

The J-bit (Jazelle support) is irrelevant to LLVM, and it doesn't
affect code generation, attributes, optimizations, or anything else,
apart from selecting the default CPU.

Diff Detail

Repository
rL LLVM

Event Timeline

tyomitch updated this revision to Diff 40430.Nov 17 2015, 2:22 PM
tyomitch retitled this revision from to Handle ARMv6-J as an alias, instead of fake architecture.
tyomitch updated this object.
tyomitch added reviewers: rengolin, logan, compnerd.
tyomitch added a subscriber: llvm-commits.
rengolin added subscribers: jroelofs, joerg, t.p.northover.

I'm ok with the change, but I'd rather hear from @joerg, @compnerd, @t.p.northover, @jroelofs to make sure we're not breaking anything.

cheers,
--renato

unittests/ADT/TripleTest.cpp
855 ↗(On Diff #40430)

Non-functional triples were recorded because of the default CPU they had more than what they actually meant. In this case, the difference is between having VFP or not, which makes a huge difference on soft vs hard float and could wreak havoc on build systems.

I'm not saying that it's a good idea to keep the unknown legacy, but we gotta at least give some thought to this change.

tyomitch added inline comments.Nov 18 2015, 5:55 AM
unittests/ADT/TripleTest.cpp
855 ↗(On Diff #40430)

Wow, so the difference between armv6j and armv6 triples is not Jazelle but VFP?

Couldn't be any more obscure than this...

jroelofs edited edge metadata.Nov 19 2015, 11:29 AM

GCC seems to think arm1136j-s and arm1136jz-s are real things. FWIW, it treats arm1136j-s as v6J + no VFP2, arm1136jf-s as v6J + VFP2, arm1136jz-s as v6zk + no VFP2, and arm1136jz-s as v6zk + VFP2. That being said, it's possible GCC was wrong for adding them in the first place. I have no idea how many people rely on this behavior in gcc, or in clang.

Perhaps it's best to "break" this with an error message that says: "x is not supported, did you mean y?"... that way users whose build systems are dependent on this mcpu at least get an explanation on what they have to fix, rather than them complaining that something just doesn't work anymore.

GCC seems to think arm1136j-s and arm1136jz-s are real things. FWIW, it treats arm1136j-s as v6J + no VFP2, arm1136jf-s as v6J + VFP2, arm1136jz-s as v6zk + no VFP2, and arm1136jz-s as v6zk + VFP2. That being said, it's possible GCC was wrong for adding them in the first place. I have no idea how many people rely on this behavior in gcc, or in clang.

Perhaps it's best to "break" this with an error message that says: "x is not supported, did you mean y?"... that way users whose build systems are dependent on this mcpu at least get an explanation on what they have to fix, rather than them complaining that something just doesn't work anymore.

erm... ignore this ^, I didn't read the thread carefully enough.

Jonathan, do you perhaps know if GCC treats ARMv6 and ARMv6J as the same thing or different things?

They're treated the same AFAICT.

They're treated the same AFAICT.

Would it, then, be acceptable to unify them in LLVM too, which is what this patch does?

They're treated the same AFAICT.

Would it, then, be acceptable to unify them in LLVM too, which is what this patch does?

Yeah, that's my understanding of all of this.

jroelofs accepted this revision.Nov 20 2015, 8:30 AM
jroelofs edited edge metadata.
This revision is now accepted and ready to land.Nov 20 2015, 8:30 AM
This revision was automatically updated to reflect the committed changes.

It is my understanding as well that -march=armv6 and -march=armv6j are treated the same code gen wise by GCC. However, they do cause different builtin macro to be defined. For example:

#define ARM_ARCH_6J 1

and:

#define ARM_ARCH_6 1

I don't know what the current behavior of Clang/LLVM is with respect to these macros, whether this change to unify them will impact that, or if anyone out there has code that will break due to the macros, but it is something to consider (I would probably err on the side of caution, but maybe that is too conservative).