[PATCH] D11216: [ARM] Warnings for .cpu/.fpu/.arch directives in ASM

[Renato Golin]

rengolin created this revision.
rengolin added reviewers: compnerd, t.p.northover, kristof.beyls, jmolloy, echristo.
rengolin added a subscriber: llvm-commits.
rengolin set the repository for this revision to rL LLVM.
Herald added subscribers: rengolin, aemerson.

Reuse of architecture assembly directives is not an error, but can be
misleading and bring future problems, like lack of proper checking on
bad code generation, if the latter directive is less restricting than
the former, and the latter is only valid for one function.

One example is for IFUNC implementations, having multiple architectures
per compilation unit:

    .fpu softvfp
    func_soft_float:
      ...

    func_vfp3:
      .fpu vfpv3
      ...

    @ end of IFUNC block
    ...

    .unrelated_func:
      @ assuming back to softvfp
      @ but .fpu vfpv3 leaked until here
      ...
      @ then we accidentally generate a VFP unstruction
      VMOV ...
      @ this should be an error, but the .fpu directive above masks it

The VMOV above could be a compiler code generation error, or could be a
user error, either way, we can't warn about this error, due to the leaky
nature of .fpu directives.

Another case is in inline assembly:
    global_vfp = false;
    my_c_func() {
      _asm_(".fpu vfpv3");
      // code that depends on VFP3
    }
    
    my_other_func() {
      if (global_vfp) {
        _asm_("VMOV ...");
      else
        _asm_("mov, mov...");
    }

Here, there's an implicit contract between the C global_vfp variable and
the assmebly .fpu directive. If that contract is broken, there's no way
the compiler can warn the user, since the ".fpu vfpv3" will allow the
assembler to emit the VMOV regardless of what global_vfp is. This was
probably not the intention of the programmer when (s)he used the .fpu
directive in my_c_func().

Both examples are based on a broken design, but both appear in user code
often enough to be a problem. Multi-architecture machines and OSs are
becoming more common to exacerbate that problem, so a warning is due.

Fixes PR20757.

Problems:
* This will turn unconditional warnings when using "clang ... file.s" and may break compilations where -Wall and -Werror are set.
 * I could add an -mllvm -disable-asm-warnings for now, until we can link Clang warning infrastructure with the assembler
* This warns on misuse of directives, not on instruction selection issues (as outlined by the test).
 * There's no way to warn on codegen problems without changing the flags back to what's in the header, and that's a big change in behaviour from GAS.

Repository:
  rL LLVM

http://reviews.llvm.org/D11216

Files:
  lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  test/MC/ARM/directive-warning.s

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D11216.29776.patch
Type: text/x-patch
Size: 5089 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150715/510a398e/attachment.bin>