[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv created this revision.
Herald added a subscriber: javed.absar.

(Copy/pasting the reviewer list from https://reviews.llvm.org/D26856.)

Addresses https://bugs.llvm.org/show_bug.cgi?id=30792 .

In GCC, -mgeneral-regs-only emits errors upon trying to emit floating-point or
vector operations that originate from C/C++ (but not inline assembly).
Currently, our behavior is to accept those, but we proceed to "try to form
some new horribly unsupported soft-float ABI."

Additionally, the way that we disable vector/FP ops causes us to crash when
inline assembly uses any vector/FP operations, which is bad.

This patch attempts to address these by:

- making -mgeneral-regs-only behave exactly like -mno-implicit-float to the backend, which lets inline assembly use FP regs/operations as much as it wants, and
- emitting errors for any floating-point expressions/operations we encounter in the frontend.

The latter is the more interesting bit. We want to allow declarations with
floats/vectors as much as possible, but the moment that we actually
try to use a float/vector, we should diagnose it. In less words:

  float f(float a); // OK
  int j = f(1); // Not OK on two points: returns a float, takes a float
  float f(float a) {  // Not OK: defines a function that takes a float and returns
                      // a float
    return 0; // Not OK: 0 is converted to a float.
  }

A trivial implementation of this leaves us with a terrible diagnostic
experience (e.g.

  int r() {
    int i = 0, j = 0;
    return 1.0 + i + j;
  }

emits many, many diagnostics about implicit float casts, floating adds, etc.
Other kinds of diagnostics, like diagnosing default args, are also very
low-quality), so the majority of this patch is an attempt to handle common
cases more gracefully.

Since the target audience for this is presumably very small, and the cost of not
emitting a diagnostic when we should is *a lot* of debugging, I erred on the
side of simplicity for a lot of this. I think this patch does a reasonably good
job of offering targeted error messages in the majority of cases.

There are a few cases where we'll allow floating-point/vector values to
conceptually be used:

  int i = 1.0 + 1; // OK: guaranteed to fold to an int
  
  float foo();
  int bar(int i = foo()); // OK: just a decl.
  
  int baz() {
    int a = bar(1); // OK: we never actually call foo().
    int b = bar(); // Error: calling foo().
    return a + b;
  }
  
  struct A { float b; };
  
  void qux(struct A *a, struct A *b) {
    // OK: we've been codegening @llvm.memcpys for this seemingly since 2012.
    // For the moment, this bit is C-only, and very constrained (e.g. assignments
    // only, rhs must trivially be an lvalue, ...).
    *a = *b;
  }

The vibe I got from the bug is that the soft-float incantations we currently
emit when using -mgeneral-regs-only are basically unused, so I'm unsure
if we want a flag/option that lets users flip back to the current
-mgeneral-regs-only behavior. This patch lacks that feature, but I'm happy
to add it if people believe doing so would be valuable.

One final note: this may seem like a problem better solved in CodeGen.
I avoided doing so because that approach had a few downsides:

- how we codegen any expression that might have FP becomes observable,
- checks for this become spread out across many, many places, making it really easy to miss a case/forget to add it in a new place (see the above "few users, bugs are expensive" point), and
- it seems really difficult to "look upwards" in CodeGen to pattern match these into nicer diagnostics, especially in the case of default arguments, etc.


https://reviews.llvm.org/D38479

Files:
  docs/UsersManual.rst
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Driver/CC1Options.td
  include/clang/Sema/Sema.h
  lib/Driver/ToolChains/Arch/AArch64.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExprCXX.cpp
  test/CodeGen/aarch64-mgeneral_regs_only.c
  test/Driver/aarch64-mgeneral_regs_only.c
  test/Sema/aarch64-mgeneral_regs_only.c
  test/SemaCXX/aarch64-mgeneral_regs_only.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38479.117418.patch
Type: text/x-patch
Size: 33496 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171002/7e873b28/attachment-0001.bin>