[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

Tue Jun 4 08:52:47 PDT 2019

erichkeane added a comment.

In D56571#1523895 <https://reviews.llvm.org/D56571#1523895>, @nickdesaulniers wrote:

> Was this committed accidentally?
>
> Today in master I see:
>
> - r362106: Revert "clang support gnu asm goto." Erich Keane <erich.keane at intel.com>
> - r362059: Mark CodeGen/asm-goto.c as x86 specific after r362045 Fangrui Song <maskray at google.com>
> - r362045: clang support gnu asm goto. Jennifer Yu <jennifer.yu at intel.com>
>
>   and yet this Phabricator review is still open.
>
>   It may be easier to rebase this into a monorepo checkout, then commit with `git llvm push`. https://llvm.org/docs/GettingStarted.html#for-developers-to-commit-changes-from-git
>
>   As @kees  noted, for new test files that contain x86 assembly, they need a `-triple` that specifies an x86 target, otherwise the implicit target is based on the host, which may not be x86.  We still want to verify that non-x86 host can cross compile to x86 correctly.  Sorry we did not catch this earlier in code review.

I think Jennifer just forgot the Differential Revision line, so this was never closed.  The test failures resulted in her wanting the patch reverted until she could fix it (hence my revert)

In D56571#1529349 <https://reviews.llvm.org/D56571#1529349>, @xbolva00 wrote:

> There is a bug.. I took GCC’s example for asm goto and trunk emits:
>  https://godbolt.org/z/9EcTR9
>
> Wrong jb instruction target..

Hmm... I don't see the jb instruction target name reflected in the IR, so I suspect @craig.topper should be made aware of the descrepancy (author of https://reviews.llvm.org/D53765).

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

https://reviews.llvm.org/D56571