This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by sanwou01 on Mar 21 2017, 11:08 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

sanwou01 created this revision.Mar 21 2017, 11:08 AM
rengolin edited edge metadata.Mar 21 2017, 1:54 PM

Why not just use -mllvm for this one?

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

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 updated this revision to Diff 92643.Mar 22 2017, 8:53 AM

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

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

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 accepted this revision.Mar 23 2017, 4:57 AM
samparker added a subscriber: 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

This revision is now accepted and ready to land.Mar 23 2017, 4:57 AM
This revision was automatically updated to reflect the committed changes.