LLVM issuse:AArch64 TargetParser

Jojo_Ma · May 5, 2016, 10:11am

Hi everyone,

I’m a member engineer of linaro’s llvm team,coming from Spreadtrum.I am a new person on LLVM.Now I’m writing a Target Parser for AArch64,so options parsing of AArch64 about cpu & arch & fpu can be summary to one place.

In the TargetParser,we assume “aarch64” and “arm64” are synonyms of armv8a(as they are only for armv8a,people usually do this). So after using AArch64TargetParser in options parsing in Clang,using “-march=aarch64” and “-march=arm64” are both valid.

I saw Kevin has a different view on this in his submission:r213353.

+// RUN: %clang -target aarch64 -march=arm64 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-ARCH-NOT-ACCEPT %s
+// RUN: %clang -target aarch64 -march=aarch64 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-ARCH-NOT-ACCEPT %s
+// CHECK-ARCH-NOT-ACCEPT: error: the clang compiler does not support

So I failed on the regression testing.So I think we should reach a consensus on this issue.
Would you mind letting me know what you think of this issuse?

Thank you very much!Looking forward to your reply!
I enclose the attachments for your reference.

Best Regards,
Jojo.ma

AArch64TargetParser_clang.patch (4.18 KB)

AArch64TargetParser_llvm.patch (31.9 KB)

Bradley_Smith · May 5, 2016, 12:01pm

Hi,

Allowing -march=aarch64/arm64 is somewhat misleading I think, -march is used for specifying an architecture version to target whereas aarch64/arm64 don’t convey any information to that effect, does it mean armv8a, armv8.1-a, etc? Personally I’m in favor of having that be rejected, it would be good to have TargetParser/Clang generally reject CPUs/architectures that are not valid or don’t add information as opposed to converting them to a fairly arbitrary choice (that will and does only cause confusion).

Regards,

Bradley Smith

Renato_Golin1 · May 5, 2016, 4:16pm

Allowing -march=aarch64/arm64 is somewhat misleading I think, -march is used
for specifying an architecture version to target whereas aarch64/arm64 don’t
convey any information to that effect, does it mean armv8a, armv8.1-a, etc?

Hi Bradley,

That's a good point. But also, what does "armv8a" mean? AArch64? AArch32?

I guess we could use the triple (which has aarch64 or arm in it), but
then it will need the triple or else we'll have a problem, since the
current interpretation of "armv8a" is AArch32.

Personally I’m in favor of having that be rejected, it would be good to have
TargetParser/Clang generally reject CPUs/architectures that are not valid or
don’t add information as opposed to converting them to a fairly arbitrary
choice (that will and does only cause confusion).

Rejecting that on -march is independent than rejecting that in the
target parser.

I think the logic of refusing this or that "arch" string in this or
that tool should be restricted to the tool, not the parser.

Means, we still have to worry about the parser's ability to understand
armv8+ with respect to AArch64 vs AArch32.

I could suggest we use -m32/-m64, but I won't.

cheers,
--renato

Jojo_Ma · May 6, 2016, 7:15am

Hi Bradley & Renato,

Thank you very much!

Allowing -march=aarch64/arm64 is somewhat misleading I think, -march is used
for specifying an architecture version to target whereas aarch64/arm64 don’t
convey any information to that effect, does it mean armv8a, armv8.1-a, etc?

That’s a good point. But also, what does “armv8a” mean? AArch64? AArch32?

I guess we could use the triple (which has aarch64 or arm in it), but
then it will need the triple or else we’ll have a problem, since the
current interpretation of “armv8a” is AArch32.

I agree with you two. Using -march=aarch64/arm64 is a bit misleading,as Bradley said.
I did some tests on the things renato mentioned.Only when we specified aarch64 in the triple,
-march=armv8+ be interpreted to AArch64,otherwise AArch32.

Personally I’m in favor of having that be rejected, it would be good to have
TargetParser/Clang generally reject CPUs/architectures that are not valid or
don’t add information as opposed to converting them to a fairly arbitrary
choice (that will and does only cause confusion).

Rejecting that on -march is independent than rejecting that in the
target parser.

I think the logic of refusing this or that “arch” string in this or
that tool should be restricted to the tool, not the parser.

Means, we still have to worry about the parser’s ability to understand
armv8+ with respect to AArch64 vs AArch32.

I could suggest we use -m32/-m64, but I won’t.

Actually,I found there is a same problem for arm.For this case,I think maybe we can play a trick in the clang.
Checking whether the given arch valid or not,before we throw it to the parser,which can be used for both arm
and aarch64.

Jojo_Ma · May 9, 2016, 8:19am

Hi all,

Actually,I found there is a same problem for arm.For this case,I think maybe we can play a trick in the clang.
Checking whether the given arch valid or not,before we throw it to the parser,which can be used for both arm
and aarch64.

For the actions I mentioned above,I wrote a check function as below, basing on the naming rules of the arm architecture.

+//Only if -march startwith “armv” or “v” or “iwmmxt” or “xscale”,it can be seen valid and sended to TargetParser
+//for further parsing
+static bool checkArchValid(StringRef Arch)
+{

if ((Arch.startswith(“armv”)) || Arch[0] == ‘v’ ||
(Arch.startswith(“iwmmxt”)) || (Arch.startswith(“xscale”)))
return true;
return false;
+}

I just do a simple check in clang.As the TargetParser will parse it in detail,if passed.
If you have any suggestions, please let me know.Thank you very much!

Attachments are the newest patches.

Regards,

Jojo

AArch64TargetParser_llvm.patch (31.9 KB)

AArch64TargetParser_clang.patch (5.24 KB)

Renato_Golin1 · May 9, 2016, 8:39am

Hi Jojo,

With the TargetParser, the idea is to avoid that specific knowledge on
the tools themselves, and moving all string parsing (startswith,
StringSwitch) to the parser.

But that creates the conundrum that we've seen, that we can't parse it
uniquely without more context. It also seems that we need the Triple
to be able to infer anything about march. That is the logic that needs
to be in Clang, not the string parsing.

Another alternative would be to create two checkValid functions, one
explicitly for ARM and one for AArch64, and call them from different
places in Clang.

Hope that helps.

cheers,
--renato

Jojo_Ma · May 9, 2016, 9:53am

Hi renato,

With the TargetParser, the idea is to avoid that specific knowledge on
the tools themselves, and moving all string parsing (startswith,
StringSwitch) to the parser.

But that creates the conundrum that we’ve seen, that we can’t parse it
uniquely without more context.

This is just as a filter,it should not affect the context,I think.
But in order to filter out the invalid arch, I have no idea how to avoid doing the string parsing in the tool for now.

It also seems that we need the Triple
to be able to infer anything about march. That is the logic that needs
to be in Clang, not the string parsing.

Feature parsing is done after the triple identified. As we can see below：

static void getTargetFeatures(const ToolChain &TC, const llvm::Triple &Triple,
const ArgList &Args, ArgStringList &CmdArgs,
bool ForAS) {
… …
case llvm::Triple::arm:
case llvm::Triple::armeb:
case llvm::Triple::thumb:
case llvm::Triple::thumbeb:
getARMTargetFeatures(TC, Triple, Args, Features, ForAS);
break;
… …
case llvm::Triple::aarch64:
case llvm::Triple::aarch64_be:
getAArch64TargetFeatures(D, Args, Features);
break;
… …
}

Another alternative would be to create two checkValid functions, one
explicitly for ARM and one for AArch64, and call them from different
places in Clang.

Yes, it will be more clear.

Thank you very much! Renato.

Regards,

Jojo

Bruce_Hoult · May 18, 2016, 12:17pm

Note that armv8a modifies the A32 and T32 instruction sets, and is therefore an important -march option for 32 bit code. Therefore armv8a can not be used to imply aarch64.

For example:

IT predication instruction with more than a single controlled 16 bit instruction, or an instruction that modifies PC is deprecated. All old uses of IT work by default, but a control bit enables the OS to cause a trap on use of a deprecated use.
new A32 and T32 Load Acquire/Store Release instructions
new A32 and T32 scalar FP instructions including conditional select, min, max, new rounding modes for FP to int conversion, conversion between half precision and double precision
new A32 and T32 vector forms of the above FP operations
new A32 and T32 cryptographic instructions
changes to barrier instructions

Users can reasonably expect -march=armv8a for 32 bit code to potentially generate the new FP instructions and restrict the use of the IT instruction.

James_Molloy · May 18, 2016, 12:21pm

Hi,

A64 versus A32/T32 code generation is controlled by the -target option which I don’t believe is under discussion here.

James

Renato_Golin1 · May 18, 2016, 12:31pm

Indeed, that is the case. The -target option picks the back-end, and
-march chooses the support level, same as GCC.

Just to be clear:

"-target armv8" implies AArch32, so you can't use "-march=aarch64" with it.
"-target aarch64" is strictly AArch64, so you can't use
"-march=armv7" or lower with it.

However, the first case above is an *aberration* and the recommended usage is:

"-target arm -march=armv8a" which will select the "ARMv8A AArch32
ARM instruction set".
"-target aarch64 -march=armv8a" which will select the "ARMv8A
AArch64 instruction set".

Also, please do *not* use:

"-target thumb -march=armv8a", even though it works today in LLVM,
it's another aberration.

Instead, use:

"-target arm -march=armv8a -mthumb". This will correctly choose the
"ARMv8A AArch32 Thumb2 instruction set".

cheers,
--renato

Bruce_Hoult · May 18, 2016, 12:46pm

Thanks!

Just for completeness, if you want to force T16, should you use

“-target arm -march=armv6 -mthumb -mfloat-abi=soft”

Or is there a better way?

(the last option needed because T16 has no access to FP registers, so you’d better either not be using FP arguments/results at all or else compile everything else you link to with softfp as well)

Renato_Golin1 · May 18, 2016, 12:54pm

That should be enough, yes. Choosing -march=armv6m is another way, but
that will probably change the codegen profile, too.

Depends on what you want.

cheers,
--renato