[PATCH] D69876: Allow output constraints on "asm goto"

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers requested changes to this revision.
nickdesaulniers added a comment.
This revision now requires changes to proceed.

Thinking hard about this, I think there's four cases we really need to think hard about:

1. asm redirects control flow BEFORE assigning to output. ie.

  int foo(void) {
    int y;
    asm volatile goto ("ja %l1" : "=r"(y) ::: err);
    return y;
  err:
    return y;
  }

I think we can chalk this up to user error; stupid asm in, stupid asm out.  If the inline asm never assigns to the output, regardless of how the assembly block is exited, there's nothing we can do.  Your child patch (https://reviews.llvm.org/D71314) warns in the second return, though the first return is equally invalid.

2. asm redirects control flow AFTER assigning to output. ie.

  int foo(void) {
    int y;
    asm volatile goto ("mov 42, %0; ja %l1" : "=r"(y) ::: err);
    return y;
  err:
    return y;
  }

Why is this not valid in the case the indirect branch is taken?

The two other cases I can think of are in regards to conflicting constraints.  Consider for example the x86 machine specific output constraints (https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html#Machine-Constraints):

3. shared indirect destinations with differing/conflicting constraints

  void foo(void) {
      int y;
      asm volatile ("mov 42, %0" : "=d"(y));
  }
  void bar(void) {
      int y;
      asm volatile ("mov 42, %0" : "=c"(y));
  }
  void baz(void) {
      int y;
      asm volatile goto ("mov 42, %0; ja %1": "=d"(y) ::: quux);
      asm volatile goto ("mov 42, %0; ja %1": "=c"(y) ::: quux);
      return;
  quux:
      return;
  }

`foo` puts `42` in `%edx`. `bar` puts `42` in `%ecx`.  Where should `baz` put `42`? Your current patch set produces: `error: Undefined temporary symbol`.

4. conflicts between `register` variables and output constraints.

  void foo(void) {
      register int y asm("edx") = 0;
      asm volatile goto ("mov 42, %0; ja %l1" : "=c"(y) ::: bar);
  bar:
      return;
  }
  void baz(void) {
      register int y asm("edx") = 0;
      asm volatile ("mov 42, %0" : "=c"(y));
  }

The output constraint for `asm goto` says put the output in `%ecx`, yet the variable was declared as having register storage in `%edx`.

Looks like Clang already has bugs or at least disagrees with GCC (for the simpler case of `baz`, but the problem still exists for `foo`). Filed https://bugs.llvm.org/show_bug.cgi?id=44328.

I need to think more about this, but I feel like people may see #2 as a reason to not use this feature and I have serious concerns about that.



================
Comment at: clang/docs/LanguageExtensions.rst:1275
+It's important to note that outputs are valid only on the "fallthrough" branch.
+For example, the value assigned to `y` is not valid in the above `err` block.
+
----------------
Would you mind removing the `above` in this sentence. I initially parsed this as `the block above err`, which is not correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876