This is an archive of the discontinued LLVM Phabricator instance.

Fix default processor name for armv6k.
Needs RevisionPublic

Authored by LordHoto on Mar 11 2016, 7:12 AM.

Details

Summary

The former processor name "arm1176j-s" does not exist according to http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.set.arm11/index.html and causes issues when actually being used during compilation (I tried to compile libffi 3.2 with clang for old iOS, which defaults to "armv6k", and it failed because invalid instructions were used).

It looks like r246930 did introduce the use of that name. The comments on the diff (D11590) for the revision suggest that it was known that this CPU name is invalid and suggest they removed it. However, the wrong processor name still ended up in the finally committed patch.

I reverted the processor name back to the former default "arm1176jzf-s" and introduced a new entry for "armv6k" without VFP which uses "arm1176jz-s", which are the first processor to support ARMv6k extensions starting from their first revision as far as I can tell from the docs on arm.com.

This sadly causes some regressions in the Clang unit tests. Clang appareantly assumes when you target CPU "arm1176jzf-s" you automatically target architecture "armv6kz". This looks like a sane thing, but now breaks because CPU "arm1176jzf-s" is used for both architectures. As a result, "armv6k" is targetted now when targetting CPU "arm1176jzf-s". I have some patch for Clang which marks the respective unit tests as FIXME and correctly adepts the tests for default processor for "armv6k". I will put it in a diff in case this one makes it.

Diff Detail

Event Timeline

LordHoto updated this revision to Diff 50425.Mar 11 2016, 7:12 AM
LordHoto retitled this revision from to Fix default processor name for armv6k..
LordHoto updated this object.
LordHoto added a subscriber: llvm-commits.
ismail added a subscriber: ismail.Mar 16 2016, 5:05 AM

This breaks two tests:

/home/abuild/rpmbuild/BUILD/llvm-3.8.0.src/tools/clang/test/Driver/arm-cortex-cpus.c:76:15: error: expected string not found in input
// CHECK-V6K: "-cc1"{{.*}} "-triple" "armv6k-{{.*}} "-target-cpu" "arm1176j-s"

/home/abuild/rpmbuild/BUILD/llvm-3.8.0.src/tools/clang/test/Driver/biarch.c:11:11: error: expected string not found in input
// ARMV6: "-cc1" "-triple" "armv6kz--netbsd-eabihf"

Yup those are the Clang tests I talk about. The first one is incorrect already because it relies on the wrong CPU name. The second one is a bit more tricky, because now the same default CPU is used for armv6kz and armv6k, which makes the code pick whatever is first and not whatever features a richer instruction set.

I noticed the first revision to introduce this odd processor name was r246930 by Renato Golin. Rento do you remember why you used this processor name? Can you review this patch and give some feedback? I would really like to get this upstream.

rengolin resigned from this revision.Oct 4 2016, 5:24 AM
rengolin edited reviewers, added: richard.barton.arm, labrinea, silviu.baranga, jmolloy; removed: rengolin.

Sorry, completely missed that one.

I'll let the ARM folks check the CPU names. I have to say my ARMv6 history faded away years ago, but I remember some unofficial names leaking through GCC.

labrinea edited edge metadata.Oct 7 2016, 7:17 AM

@LordHoto can you be more specific when saying that invalid instructions are generated when targeting armv6k? I am afraid that using the same cpu-name as the key for two different entries of the table is a wrong approach.

falstaff84 requested changes to this revision.Mar 20 2018, 12:34 PM
This comment was removed by falstaff84.
This revision now requires changes to proceed.Mar 20 2018, 12:34 PM

I seem to have a related issue: I am using -march=armv6k and -no-integrated-as, this generates the following output:

	.text
	.syntax unified
	.eabi_attribute	67, "2.09"	@ Tag_conformance
	.cpu	arm1176jz-s
	.eabi_attribute	6, 6	@ Tag_CPU_arch
	.eabi_attribute	8, 1	@ Tag_ARM_ISA_use
	.eabi_attribute	9, 1	@ Tag_THUMB_ISA_use
	.fpu	vfpv2
	...

With that the GNU assembler subsequently errors out:

/tmp/empty-bc2ea3.s: Assembler messages:
/tmp/empty-bc2ea3.s:4: Error: unknown cpu `arm1176j-s'

According to the binutils documentation arm1176j-s is not a valid CPU. Using -mtune=arm1136jz-s does not help. But armv6kz seems to work (uses cpu arm1176jzf-s).

Looks like this is preventing compiling the Linux kernel with Clang for armv6: https://github.com/ClangBuiltLinux/linux/issues/55.

I seem to have a related issue: I am using -march=armv6k and -no-integrated-as, this generates the following output:
/tmp/empty-bc2ea3.s:4: Error: unknown cpu `arm1176j-s'

Right, this is not "invalid instruction", just "unknown cpu", which is totally different. :)

I thought we had removed the 1176j-s back then, but apparently not.

@labrinea an alternative fix would be to make 1176jz-s the default for armv6k as well as armv6kz.

But honestly, I lost track of all the horrible names armv6 used to have, and the information is slightly different everywhere.

To me, it doesn't sound like an important enough difference to matter. Does LLVM treat j-s and jz-s differently?

Finally, the list GNU uses can be different than the list Arm uses which, in turn, is different to the list LLVM uses.

The Arm docs are not consistent enough to be authoritative either, so it has never been clear cut.

srhines added inline comments.
include/llvm/Support/ARMTargetParser.def
182

This uses the same name as line 184, which seems like a bug to me.

rengolin added inline comments.Sep 13 2018, 12:48 PM
include/llvm/Support/ARMTargetParser.def
182

I think that's what @labrinea was talking about.

Can anyone from Arm confirm what "j-s" really is, if it is anything at all?

I had so many conversations about this in the past and all of them got me even more confused than before.

There isn't a processor called ARM1176J-S, only ARM1176JZ-S and ARM1176JZF-S. I've already left a comment about J and S. Z is for Trustzone and F is for hardware floating point. There isn't a good external source for all the possible arm 11 CPU names unfortunately. There is a page of trademarks that contains all the ones that I know about https://www.arm.com/company/policies/trademarks/arm-trademark-list/arm11-trademark.

include/llvm/Support/ARMTargetParser.def
182

The j stands for Jazelle (https://en.wikipedia.org/wiki/Jazelle) which was a Thumb like mode that could be entered with I think the BXJ mode. This would allow the processor to execute a handful of the most common Java bytecodes. The only affect that this would have on LLVM would potentially be to enable the BXJ instruction. Personally I don't think it is worth it given that direct execution of bytecodes lost out to JIT compilers so I don't expect anyone actually wanting to use it.

The s stands for synthesizable. This means that the design was available to hardware licensees in VHDL rather than as a fixed macrocell. Obviously this makes no difference to code-generation.

If j-s doesn't exists (remember, most of that list came from the ancient days of llvm, so could very easily be completely wrong, but "works"), I all for removing it and making a new (correct) CPU name as the default for armv6k.

I'm also happy to follow gcc here (least surprise), given the consequences are very very small, if any, to code generation.

include/llvm/Support/ARMTargetParser.def
182

Sorry Peter, I should have been more clear. I meant what is "j-s", armv6, 6k, 6kz?

But you answered my question, it isn't. Any of them.

So, the natural question here is: what is the default for armv6k?

I think the natural default CPU for ARMv6K is the "mpcore". The K stands for kernel extensions and includes the load,store and clear exclusive instructions but otherwise no effect on code generation. I would expect that ARMv6KZ includes the instructions for trustzone such as SMC (secure monitor call) but again, this doesn't have an effect on code generation.

include/llvm/Support/ARMTargetParser.def
182

Looking at ARM Compiler 5 which has quite a good mapping between the Architectures and CPU names. I get:

ARM1136J-SARMv6
ARM1136JF-SARMv6
ARM1136J-S-rev1ARMv6K
ARM1136JF-S rev1ARMv6K
ARM11MPCoreARMv6K
ARM1176JZ-SARMv6KZ
ARM1176JZF-SARMv6KZ

The Arm1136 TRM (Technical Reference Manual ) http://infocenter.arm.com/help/topic/com.arm.doc.ddi0211h/DDI0211.pdf has:

The ARMv6k architecture features were introduce in the rev1 (r1p0) release of the ARM1136JF-S processor. This means  that the LDREXB, LDREXH, LDREXD, STREXB, STREXH, STREXD, and CLREX instructions are only available from the rev1 (r1p0) release of the processor.

To summarise. If we are being safe then we say arm1136jf-s and arm1136j-s are ARMv6 (GCC does this). The next CPU down the list that is always ARMv6k is mpcore.

Looking at the code I think:
Line 181 should be removed as the CPU doesn't exist (gcc doesn't support it either).
ARM_CPU_NAME("arm1136jz-s", AK_ARMV6, FK_NONE, false, AEK_NONE)

Line 182 should be removed as arm1176jz-s is incorrectly given AK_ARMV6K here but appears correctly on line 184.
ARM_CPU_NAME("arm1176jz-s", AK_ARMV6K, FK_NONE, false, AEK_NONE)

Line 183 should be removed as arm1176jzf-s is incorrectly given AK_ARMV6K here but appears correctly on line 187.
ARM_CPU_NAME("arm1176jzf-s", AK_ARMV6K, FK_VFPV2, true, AEK_NONE)

Corrections are always welcome. Please submit an update to this thread (or a new patch) to fix the changes you propose.

Thanks!

If no-one else beats me to it, I'll take a look when I get back from a conference (24th September).

I've created D52594 LLVM and D52595 dependent clang test update with my suggested changes. I can't upload the diff to this review unfortunately.

The other two reviews were approved. This one can now be closed. Thanks!

Thanks. I'll commit the other two tomorrow when I get into work and close this one out.

I've committed D52594 and D52595. I can't abandon this revision as I didn't start it, but logically this review is closed.

Herald added a project: Restricted Project. · View Herald TranscriptMay 24 2023, 4:54 AM