[ARM] Add a driver option for +no-neg-immediates

[rengolin]

Why not just use -mllvm for this one?

[sanwou01]

Hi Renato,

I'm not sure what you have in mind. The command clang -target arm-none-gnueabi foo.s -mllvm -mattr=+no-neg-immediates does not seem to work for me. Am I missing something?

Also, this seems to suggest that I shouldn't be using this construct.

As this is not really a "feature" of the architecture, so a modifier to -march (like -march=armv8+dsp) doesn't make sense.

Note also that -mno-neg-immediates also affects inline assembly, so it makes sense as a compiler option.

Thanks,
Sanne

[rengolin]

Hi Sanne,

My point is that I think this option is not meant for user consumption, so it doesn't deserve a Clang option.

If it's useful for creating tests, or front-ends to control the target behaviour, then the target-feature is already working as expected.

The -mllvm should work if the option exists in the ARM back-end only (ie works on llc and alternatives).

Maybe it just doesn't work with -mattr? Have you tried directly?

All in all, unless there is a clear use case for normal users to be using this in a production environment, there is no point in passing this through Clang.

Or maybe I just don't understand what the case is, so if there is one, maybe explaining would "fix all". :)

cheers,
--renato

[sanwou01]

Fix a typo in the test; add the option for the Aarch64 target as well.

[sanwou01]

Hi Renato,

To expand on the use-case for this option, this is aimed at developers who want their assembly code to correspond *exactly* to what is encoded in the object file. An example of this is in the development of safety critical software. The same goes for -mimplicit-it=never and friends, which is already an option for clang.

I hope that sufficiently clarifies the motivation for this option.

Thanks,
Sanne

[rengolin]

Right, I can see the use case, but I'm still not convinced it's worth the flag. I'm also not against it, so I'll let for someone else to approve.

The code itself looks good.

cheers,
--renato

[samparker]

Hi Sanne,

The code LGTM and I can't think of a more suitable way of doing this as it fits well with other similar options that we provide.

cheers,
sam

cfe/trunk/include/clang/Driver/Options.td

 def ffixed_r9 : Flag<["-"], "ffixed-r9">, Group<m_arm_Features_Group>,
   HelpText<"Reserve the r9 register (ARM only)">;
 def mno_movt : Flag<["-"], "mno-movt">, Group<m_arm_Features_Group>,
   HelpText<"Disallow use of movt/movw pairs (ARM only)">;
 def mcrc : Flag<["-"], "mcrc">, Group<m_arm_Features_Group>,
   HelpText<"Allow use of CRC instructions (ARM only)">;
 def mnocrc : Flag<["-"], "mnocrc">, Group<m_arm_Features_Group>,
   HelpText<"Disallow use of CRC instructions (ARM only)">;
+def mno_neg_immediates: Flag<["-"], "mno-neg-immediates">, Group<m_arm_Features_Group>,
+  HelpText<"Disallow converting instructions with negative immediates to their negation or inversion.">;
 
 def mgeneral_regs_only : Flag<["-"], "mgeneral-regs-only">, Group<m_aarch64_Features_Group>,
   HelpText<"Generate code which only uses the general purpose registers (AArch64 only)">;
 
 def mfix_cortex_a53_835769 : Flag<["-"], "mfix-cortex-a53-835769">,
   Group<m_aarch64_Features_Group>,
   HelpText<"Workaround Cortex-A53 erratum 835769 (AArch64 only)">;
 def mno_fix_cortex_a53_835769 : Flag<["-"], "mno-fix-cortex-a53-835769">,

cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp

void aarch64::getAArch64TargetFeatures(const Driver &D, const ArgList &Args,
 
   if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
                                options::OPT_munaligned_access))
     if (A->getOption().matches(options::OPT_mno_unaligned_access))
       Features.push_back("+strict-align");
 
   if (Args.hasArg(options::OPT_ffixed_x18))
     Features.push_back("+reserve-x18");
+
+  if (Args.hasArg(options::OPT_mno_neg_immediates))
+    Features.push_back("+no-neg-immediates");
 }

cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp

void arm::getARMTargetFeatures(const ToolChain &TC,
   // for reserving r9 on ARM though (defined as a platform-specific register
   // in ARM EABI).
   if (Args.hasArg(options::OPT_ffixed_r9))
     Features.push_back("+reserve-r9");
 
   // The kext linker doesn't know how to deal with movw/movt.
   if (KernelOrKext || Args.hasArg(options::OPT_mno_movt))
     Features.push_back("+no-movt");
+
+  if (Args.hasArg(options::OPT_mno_neg_immediates))
+    Features.push_back("+no-neg-immediates");
 }
 
 const std::string arm::getARMArch(StringRef Arch, const llvm::Triple &Triple) {
   std::string MArch;
   if (!Arch.empty())
     MArch = Arch;
   else
     MArch = Triple.getArchName();

cfe/trunk/test/Driver/arm-no-neg-immediates.c

+// RUN: %clang -target arm-none-gnueabi -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT
+// RUN: %clang -target arm-none-gnueabi -mno-neg-immediates -### %s 2>&1 | FileCheck %s
+
+// RUN: %clang -target aarch64-none-gnueabi -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT
+// RUN: %clang -target aarch64-none-gnueabi -mno-neg-immediates -### %s 2>&1 | FileCheck %s
+
+// CHECK: "-target-feature" "+no-neg-immediates"
+// CHECK-DEFAULT-NOT: "+no-neg-immediates"