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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to