[asm goto] support outputs along indirect branches

[josephcsible]

Consider this C code:

int main(void) {
    int x = 123;
    asm goto("mov %1, %0\n\tjmp %l[label]" : "=r" (x) : "r" (45) : : label);
    x = 6;
    label:
    return x;
}

It's supposed to return 45. But when you compile it in Clang, it returns 123 instead. I tested this on both trunk (01b52f7) and 13.0.0, and both with -O3 and without optimizations, and the bug happens in all of those cases.

https://godbolt.org/z/vKeYrfYY5

[llvmbot]

@llvm/issue-subscribers-clang-codegen

[efriedma-quic]

Outputs are only valid on the fallthrough codepath; on other paths, the output values are ignored. That's by design. Or at least, it's a practical consequence of the way asm goto is currently implemented.

This is documented at https://clang.llvm.org/docs/LanguageExtensions.html#asm-goto-with-output-constraints . -Wuninitialized prints a warning for your testcase. The warning could probably be improved. Not sure what else we can do here.

[josephcsible]

Okay, I see what happened. This part of that documentation is no longer quite accurate:

In addition to the functionality provided by GCC’s extended assembly, clang supports output constraints with the goto form.

The goto form of GCC’s extended assembly allows the programmer to branch to a C label from within an inline assembly block. Clang extends this behavior by allowing the programmer to use output constraints:

Because as of version 11.1.0 (specifically, commit e3b3b59), GCC now supports asm goto with outputs, without a restriction to only the fallthrough codepath. So now we support only a subset of their functionality rather than an extension of it. In light of this, can this issue be reconsidered as a feature request to match that functionality?

[nickdesaulniers]

cc @jyknight @gwelymernans @isanbard

can this issue be reconsidered as a feature request to match that functionality?

Yeah, we probably can't focus on this for a couple months, but this is surely a portability issue that folks will begin to notice more and more frequently.

We'll probably need to undo parts of https://reviews.llvm.org/D79794, making INLINEASM_BR a terminator again. We'll also probably need to split critical edges to insert COPYs for the indirect target. I forget now why we didn't; I think critical edge splitting is fallible.

Also, there is one undesirable side effect where address of label would no longer refer to the same label as the asm goto IIRC. But I'd say support for outputs along all edges is probably more worthwhile than such a minute detail.

[isanbard]

If GCC's able to allow outputs on indirect branches, then they must not care about the label addresses like we did. (Segher mentioned something along those lines in our discussions with him before the implementation.) That would take care of the vast majority of issues we faced with splitting critical edges.

I don't know if it's worth making INLINEASM_BR a terminator again, since that has its own issues. We'll have to see though.

[nickdesaulniers]

https://discourse.llvm.org/t/rfc-syncing-asm-goto-with-outputs-with-gcc/65453/4

[nickdesaulniers]

Updating docs to make note of this difference https://reviews.llvm.org/D135818. Will need to revisit those docs when this is implemented.

[nickdesaulniers]

1. https://reviews.llvm.org/D135997
2. https://reviews.llvm.org/D140166
3. https://reviews.llvm.org/D139861
4. https://reviews.llvm.org/D139872
5. https://reviews.llvm.org/D139883
6. https://reviews.llvm.org/D139970
7. https://reviews.llvm.org/D140160
8. https://reviews.llvm.org/D137113
9. https://reviews.llvm.org/D136497