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.ke...@intel.com>
> - r362059: Mark CodeGen/asm-goto.c as x86 specific after r362045 Fangrui Song 
> <mask...@google.com>
> - r362045: clang support gnu asm goto. Jennifer Yu <jennifer...@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



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

Reply via email to