[PATCH] D60943: Delay diagnosing "n" constraint until after inlining

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In D60943#1474926 <https://reviews.llvm.org/D60943#1474926>, @rsmith wrote:

> In D60943#1474899 <https://reviews.llvm.org/D60943#1474899>, @void wrote:
>
> > Here's the motivating bug report: https://bugs.llvm.org/show_bug.cgi?id=41027
>
>
> Thanks, that's illuminating. OK, if we want to support that code, then there's really not much validation we can do in the frontend for constraints that require immediate constants. Is `"n"` really special in this regard, or is it just the first one that we've encountered?


I think it's just the first one we've encountered. There could be other constraints which require immediates, but they're probably target-specific. E.g. `e` and `K` for x86.

>> In general, I agree with you that diagnostics shouldn't depend on optimization levels, but inline assembly subverts this paradigm because it was originally a gcc extension. :-(
> 
> There is a question of how much weird stuff we should accept in the name of GCC compatibility. In this case, the fact that the frontend rejects seems like the tip of an iceberg: the code in the PR is also relying on the control flow of the `&&` inside the `if` to be emitted in a very particular way (or to be optimized to that form), and for dead-code elimination to be considered and to actually fire, and probably more things besides. Do we at least still produce a nice diagnostic if the value ends up not being a constant?

It does, but for the wrong line:

  [morbo at fawn:llvm] cat bad.c
  static __attribute__((always_inline)) void outl(unsigned port, unsigned data) {
    asm volatile("outl %0,%w1" : : "a"(data), "n"(port));
  }
  
  void f(unsigned port) {
    outl(port, 1);
  }
  [morbo at fawn:llvm] ./llvm.opt.install/bin/clang -c bad.c
  bad.c:2:16: error: constraint 'n' expects an integer constant expression
    asm volatile("outl %0,%w1" : : "a"(data), "n"(port));
                 ^
  1 error generated.

I'll need to modify the LLVM patch to output the correct location.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60943/new/

https://reviews.llvm.org/D60943