Correct test for ARM targets

[DazedOwl]

Summary

Test fails on ARM targets due to ABI name between define and void. Added reg ex to skip.

[DazedOwl]

Hi, apologies for not knowing the correct procedure for selecting reviewers (I'd be grateful if anyone wants to put me straight on this), I've just selected everyone who seems to have an interest.

It's a minute change so if anyone can give it the go ahead, please do.

Thanks
Glenn

[rengolin]

Where is this failing?

It's certainly passing on all the ARM bots...

http://llvm.validation.linaro.org/

[rengolin]

Just FYI, when you submit a patch, use "git diff -U999" so that we can see the context in the web interface. Makes it easier to review. :)

[tejohnson]

I suppose this could also be fixed by adding a target triple so the ABI name is not added? Looks like most tests in that directory have one, which is presumably why they don't hit this. Unless we specifically want to test the handling of ARM bitcode here, can you do that instead?

[rengolin]

I don't see why we should force the target, given that this is a CodeGen test, should worr (and be tested) on all targets.

I'm just curious as to where it fails, and how to make it fail on the bots, so that we can catch it again.

The fix is not wrong for the kind of problem described, I just can't see it happening.

--renato

[DazedOwl]

Hi

Using a Linux Clang -cc1 version 3.4 based on LLVM 3.4 default target x86_64-pc-linux-gnu to build a simple test function:

clang -cc1 -emit-llvm -v -o loop.bc -triple arm7a-arm loop.cpp

we get:

define arm_aapcscc void @_Z4testv() #0 {

It's 'arm-aapcscc' that's giving us the problem.

[tejohnson]

In D33410#783807, @DazedOwl wrote:

Hi

Using a Linux Clang -cc1 version 3.4 based on LLVM 3.4 default target x86_64-pc-linux-gnu to build a simple test function:

That's a much older compiler than this test. Are you seeing this fail using a compiler of the same vintage as the test which was added a few months ago? I tried using the below triple on this test case (using current HEAD) and can't get it to add the ABI name.

clang -cc1 -emit-llvm -v -o loop.bc -triple arm7a-arm loop.cpp

we get:

define arm_aapcscc void @_Z4testv() #0 {

It's 'arm-aapcscc' that's giving us the problem.

[DazedOwl]

Yes using current HEAD (rev 306279) we are still getting the same failure, for both Linux and Windows.

[rengolin]

In D33410#790422, @DazedOwl wrote:

Yes using current HEAD (rev 306279) we are still getting the same failure, for both Linux and Windows.

Can you share your environment? It is surprising that this fails for you but it's currently passing on all buildbots: ARM, x86, Linux, Windows...

[DazedOwl]

Can you share your environment? It is surprising that this fails for you but it's currently passing on all buildbots: ARM, x86, Linux, Windows...

Yes, what sort of details would actually be of use to you?

[rengolin]

Apologies, you had already sent all I needed. :)

In D33410#783807, @DazedOwl wrote:

clang -cc1 -emit-llvm -v -o loop.bc -triple arm7a-arm loop.cpp

So, when I run this like the test, without -triple, I don't get the arm_aapcscc flag. On our systems, that defaults to armv7l-unknown-linux-gnueabihf. When not specifying gnueabi*, I do get the flag.

This looks like a bad behaviour, but given the complexity of ABIs, I'm not ready yet to call that foul. :)

This change is correct in the sense that different ARM targets will have different function flags by default, and since we're not using a specific target, we need to cover all bases, regardless of ABI issues that could be lurking underneath.

LGTM. Thanks, and sorry for the delay.

--renato

[dyung]

Upstream r311707 seems to have added two tests, CodeGen/profile-sample-accurate.c and Integration/thinlto_profile_sample_accurate.c which are failing for the same reason. Can this fix be applied to those changes as well?

test/CodeGen/thinlto-emit-llvm.c

 // RUN: %clang_cc1 -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc -emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc -emit-llvm-bc -o - | llvm-dis -o - | FileCheck %s
 
-// CHECK: define void @foo()
+// CHECK: define {{.*}} void @foo()
 void foo() {
 }