[ARM] Fix NEON being enabled with soft-float

[aemerson]

Summary

Fix NEON wrongly being enabled with soft-float when targeting armv8/Cortex-A53/A57.

This was caused by r200708 which enabled the crypto feature for these cores.

[rengolin]

Will this flag also appear in AArch32 command lines?

lib/Driver/Tools.cpp

728

I'd have thought that crypto would be disabled by default without the need of a flag...

[aemerson]

Not sure what you mean? This isn't a front-end flag, just ensuring that we explicitly disable a subtarget feature from within the driver.

Amara

[rengolin]

Wouldn't getARMTargetFeatures() be called for both targets? If you call clang with -target arm and no neon, would that also add -crypto to it?

Maybe having a check on an ARM test earlier would make things clearer (like CHECK-NOT: -crypto).

[jmolloy]

Hi Amara, Renato,

Amara asked me to take a look at this too - it looks fine. The addition of
-crypto should be a noop if crypto was disabled already, and will disable it
if it would be automatically enabled in LLVM (outside the scope of the clang
driver).

Renato, could you explain your objections a little more please? I'm not sure
I fully understand.

Cheers,
James

-----Original Message-----
From: [e-mail] [mailto:cfe-commits-
[e-mail]] On Behalf Of Renato Golin
Sent: 11 February 2014 12:37
To: [e-mail]; Amara Emerson
Cc: [e-mail]
Subject: Re: [PATCH] [ARM] Fix NEON being enabled with soft-float

Wouldn't getARMTargetFeatures() be called for both targets? If you call

clang with -target arm and no neon, would that also add -crypto to it?

Maybe having a check on an ARM test earlier would make things clearer

(like CHECK-NOT: -crypto).

http://llvm-reviews.chandlerc.com/D2736

---

cfe-commits mailing list
[e-mail]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

[jmolloy]

Ah I see, I get where you're coming from. But -soft-float should be a very rarely used option I think (as it disables VFP and NEON), so I personally don't think this is much of an issue.

Cheers,

James

-----Original Message-----
From: Renato Golin [mailto:[e-mail]]
Sent: 12 February 2014 09:41
To: [e-mail]
Cc: James Molloy
Subject: Re: [PATCH] [ARM] Fix NEON being enabled with soft-float

[rengolin]

LGTM, then.

--renato

[aemerson]

Thanks guys, r201223.

Amara

lib/Driver/Tools.cpp

static void getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
   // Honor -mfpu=.
   if (const Arg *A = Args.getLastArg(options::OPT_mfpu_EQ))
     getARMFPUFeatures(D, A, Args, Features);
   if (const Arg *A = Args.getLastArg(options::OPT_mhwdiv_EQ))
     getARMHWDivFeatures(D, A, Args, Features);
 
   // Setting -msoft-float effectively disables NEON because of the GCC
   // implementation, although the same isn't true of VFP or VFP3.
-  if (FloatABI == "soft")
+  if (FloatABI == "soft") {
     Features.push_back("-neon");
+    // Also need to explicitly disable features which imply NEON.
+    Features.push_back("-crypto");
+  }
 
   // En/disable crc
   if (Arg *A = Args.getLastArg(options::OPT_mcrc,
                                options::OPT_mnocrc)) {
     if (A->getOption().matches(options::OPT_mcrc))
       Features.push_back("+crc");
     else
       Features.push_back("-crc");

test/Driver/arm-mfpu.c

 // RUN: %clang -target arm-linux-eabi -mfpu=neon %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NEON %s
 // CHECK-NEON: "-target-feature" "+neon"
 
 // RUN: %clang -target arm-linux-eabi -msoft-float %s -### -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-SOFT-FLOAT %s
 // CHECK-SOFT-FLOAT: "-target-feature" "-neon"
 
+// RUN: %clang -target armv8 %s -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ARMV8-DEFAULT-SOFT-FP %s
+// CHECK-ARMV8-DEFAULT-SOFT-FP: "-target-feature" "-neon"
+// CHECK-ARMV8-DEFAULT-SOFT-FP: "-target-feature" "-crypto"
+
 // RUN: %clang -target armv8 -mfpu=fp-armv8 %s -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-ARMV8-SOFT-FLOAT %s
 // CHECK-ARMV8-SOFT-FLOAT: "-target-feature" "+fp-armv8"
-// CHECK-ARMV8-SOFT-FLOAT: "-target-feature" "-crypto"
 // CHECK-ARMV8-SOFT-FLOAT: "-target-feature" "-neon"
+// CHECK-ARMV8-SOFT-FLOAT: "-target-feature" "-crypto"
 
 // RUN: %clang -target armv8-linux-gnueabihf -mfpu=fp-armv8 %s -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FP-ARMV8 %s
 // CHECK-FP-ARMV8-NOT: "-target-feature" "+neon"
 // CHECK-FP-ARMV8: "-target-feature" "+fp-armv8"
 // CHECK-FP-ARMV8: "-target-feature" "-neon"
 // CHECK-FP-ARMV8: "-target-feature" "-crypto"