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

[Bill Wendling via Phabricator via llvm-commits]

void added a comment.

In D69876#1791475 <https://reviews.llvm.org/D69876#1791475>, @nickdesaulniers wrote:

> 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.


Right. This is human error. Unless we can parse the inline assembly (not something I'm suggesting) we won't be able to help them here.

> 2. asm redirects control flow AFTER assigning to output. ie. ``` int foo(void) { int y; asm volatile goto ("mov 42, %0; ja %l1" : "=d"(y) ::: err); return y; err: return y; } ``` Why is this not valid in the case the indirect branch is taken?

I'm not saying that it's *always* invalid, but we won't be able to guarantee correctness during code gen. For example, if you have something like:

  int foo(int x) {
    int y;
    if (x < 42)
      asm volatile goto ("mov 42, %0; ja %l1" : "=D"(y) ::: err);
    else
      asm volatile goto ("mov 0, %0; ja %l1" : "=S"(y) ::: err);
    return y;
  err:
    return y;
  }

we can't necessarily code gen this correctly for the `err` block. There was a long thread on ways we could try to do this, but they were all very sub-standard.

> 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`.

Whatever is done for inline assembly that's not "goto" is done here. Or at least should be. I'll fix the error message. But in essence it will move the correct value into the correct register. In this case, it should move 42 into both `%edx` and `%ecx`. The temporary symbol issue is most likely unrelated.

> 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.

Both functions generate the same output for me:

  foo:                                    # @foo
  	.cfi_startproc
  # %bb.0:                                # %entry
  	#APP
  	movl	42, %edx
  	ja	.Ltmp0
  	#NO_APP
  .Ltmp0:                                 # Block address taken
  .LBB0_1:                                # %bar
  	retq
  .Lfunc_end0:
  	.size	foo, .Lfunc_end0-foo
  	.cfi_endproc
                                          # -- End function
  	.globl	baz                     # -- Begin function baz
  	.p2align	4, 0x90
  	.type	baz, at function
  baz:                                    # @baz
  	.cfi_startproc
  # %bb.0:                                # %entry
  	#APP
  	movl	42, %edx
  	#NO_APP
  	retq

Note that we're not going to be able to fix all of clang's inline assembly bugs with this feature. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876