subject:"\[PATCH\] D88195\: Remove stale assert."

[PATCH] D88195: Remove stale assert.

2020-09-24 Thread Bill Wendling via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG34ca5b3392ce: Remove stale assert. (authored by void).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88195

Files:
  clang/lib/AST/Stmt.cpp
  clang/test/Modules/Inputs/asm-goto/a.h
  clang/test/Modules/Inputs/asm-goto/module.modulemap
  clang/test/Modules/asm-goto.c


Index: clang/test/Modules/asm-goto.c
===
--- /dev/null
+++ clang/test/Modules/asm-goto.c
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c -I%S/Inputs/asm-goto 
-emit-module %S/Inputs/asm-goto/module.modulemap -fmodule-name=a -o %t/a.pcm
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c -I%S/Inputs/asm-goto 
-emit-llvm -o - %s -fmodule-file=%t/a.pcm | FileCheck %s
+#include "a.h"
+
+// CHECK-LABEL: define i32 @foo(
+// CHECK: callbr {{.*}} "=r,X,{{.*}}"(i8* blockaddress(@foo, %indirect))
+// CHECK-NEXT: to label %asm.fallthrough [label %indirect]
+
+int bar(void) {
+  return foo();
+}
Index: clang/test/Modules/Inputs/asm-goto/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/module.modulemap
@@ -0,0 +1 @@
+module a { header "a.h" export * }
Index: clang/test/Modules/Inputs/asm-goto/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/a.h
@@ -0,0 +1,13 @@
+int foo(void) {
+  int x;
+
+  asm goto(""
+   : "=r"(x)
+   :
+   :
+   : indirect);
+  x = 42;
+
+indirect:
+  return x;
+}
Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -528,7 +528,6 @@
   this->NumInputs = NumInputs;
   this->NumClobbers = NumClobbers;
   this->NumLabels = NumLabels;
-  assert(!(NumOutputs && NumLabels) && "asm goto cannot have outputs");
 
   unsigned NumExprs = NumOutputs + NumInputs + NumLabels;
 


Index: clang/test/Modules/asm-goto.c
===
--- /dev/null
+++ clang/test/Modules/asm-goto.c
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c -I%S/Inputs/asm-goto -emit-module %S/Inputs/asm-goto/module.modulemap -fmodule-name=a -o %t/a.pcm
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c -I%S/Inputs/asm-goto -emit-llvm -o - %s -fmodule-file=%t/a.pcm | FileCheck %s
+#include "a.h"
+
+// CHECK-LABEL: define i32 @foo(
+// CHECK: callbr {{.*}} "=r,X,{{.*}}"(i8* blockaddress(@foo, %indirect))
+// CHECK-NEXT: to label %asm.fallthrough [label %indirect]
+
+int bar(void) {
+  return foo();
+}
Index: clang/test/Modules/Inputs/asm-goto/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/module.modulemap
@@ -0,0 +1 @@
+module a { header "a.h" export * }
Index: clang/test/Modules/Inputs/asm-goto/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/a.h
@@ -0,0 +1,13 @@
+int foo(void) {
+  int x;
+
+  asm goto(""
+   : "=r"(x)
+   :
+   :
+   : indirect);
+  x = 42;
+
+indirect:
+  return x;
+}
Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -528,7 +528,6 @@
   this->NumInputs = NumInputs;
   this->NumClobbers = NumClobbers;
   this->NumLabels = NumLabels;
-  assert(!(NumOutputs && NumLabels) && "asm goto cannot have outputs");
 
   unsigned NumExprs = NumOutputs + NumInputs + NumLabels;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88195: Remove stale assert.

2020-09-24 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers accepted this revision.
nickdesaulniers added a comment.

In D88195#2293637 , @jyknight wrote:

> In D88195#2293597 , @nickdesaulniers 
> wrote:
>
>> In D88195#2293589 , @void wrote:
>>
>>> Clarify commit message.
>>
>> Phabricator unfortunately won't amend the review description when the commit 
>> message is updated; you'll need to manually edit the description via phab's 
>> UI for other reviewers to observe the update. :(
>
> You can see the description separately, under the "Commits" tab, but yea.
> Also, if you use `arc diff --verbatim` to upload the new review, it'll update 
> the review message at the same time.
>
>   --verbatim
>   When creating a revision, try to use the working copy commit
>   message verbatim, without prompting to edit it. When updating a
>   revision, update some fields from the local commit message.

TIL, thanks for that power up.  Thanks for the fix, Bill!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88195

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

[PATCH] D88195: Remove stale assert.

2020-09-24 Thread James Y Knight via Phabricator via cfe-commits

jyknight accepted this revision.
jyknight added a comment.
This revision is now accepted and ready to land.

In D88195#2293597 , @nickdesaulniers 
wrote:

> In D88195#2293589 , @void wrote:
>
>> Clarify commit message.
>
> Phabricator unfortunately won't amend the review description when the commit 
> message is updated; you'll need to manually edit the description via phab's 
> UI for other reviewers to observe the update. :(

You can see the description separately, under the "Commits" tab, but yea.
Also, if you use `arc diff --verbatim` to upload the new review, it'll update 
the review message at the same time.

  --verbatim
  When creating a revision, try to use the working copy commit
  message verbatim, without prompting to edit it. When updating a
  revision, update some fields from the local commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88195

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

[PATCH] D88195: Remove stale assert.

2020-09-24 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added a comment.

In D88195#2293589 , @void wrote:

> Clarify commit message.

Phabricator unfortunately won't amend the review description when the commit 
message is updated; you'll need to manually edit the description via phab's UI 
for other reviewers to observe the update. :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88195

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

[PATCH] D88195: Remove stale assert.

2020-09-24 Thread Bill Wendling via Phabricator via cfe-commits

void updated this revision to Diff 294150.
void added a comment.

Clarify commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88195

Files:
  clang/lib/AST/Stmt.cpp
  clang/test/Modules/Inputs/asm-goto/a.h
  clang/test/Modules/Inputs/asm-goto/module.modulemap
  clang/test/Modules/asm-goto.c


Index: clang/test/Modules/asm-goto.c
===
--- /dev/null
+++ clang/test/Modules/asm-goto.c
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c -I%S/Inputs/asm-goto 
-emit-module %S/Inputs/asm-goto/module.modulemap -fmodule-name=a -o %t/a.pcm
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c -I%S/Inputs/asm-goto 
-emit-llvm -o - %s -fmodule-file=%t/a.pcm | FileCheck %s
+#include "a.h"
+
+// CHECK-LABEL: define i32 @foo(
+// CHECK: callbr {{.*}} "=r,X,{{.*}}"(i8* blockaddress(@foo, %indirect))
+// CHECK-NEXT: to label %asm.fallthrough [label %indirect]
+
+int bar(void) {
+  return foo();
+}
Index: clang/test/Modules/Inputs/asm-goto/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/module.modulemap
@@ -0,0 +1 @@
+module a { header "a.h" export * }
Index: clang/test/Modules/Inputs/asm-goto/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/a.h
@@ -0,0 +1,13 @@
+int foo(void) {
+  int x;
+
+  asm goto(""
+   : "=r"(x)
+   :
+   :
+   : indirect);
+  x = 42;
+
+indirect:
+  return x;
+}
Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -528,7 +528,6 @@
   this->NumInputs = NumInputs;
   this->NumClobbers = NumClobbers;
   this->NumLabels = NumLabels;
-  assert(!(NumOutputs && NumLabels) && "asm goto cannot have outputs");
 
   unsigned NumExprs = NumOutputs + NumInputs + NumLabels;
 


Index: clang/test/Modules/asm-goto.c
===
--- /dev/null
+++ clang/test/Modules/asm-goto.c
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c -I%S/Inputs/asm-goto -emit-module %S/Inputs/asm-goto/module.modulemap -fmodule-name=a -o %t/a.pcm
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c -I%S/Inputs/asm-goto -emit-llvm -o - %s -fmodule-file=%t/a.pcm | FileCheck %s
+#include "a.h"
+
+// CHECK-LABEL: define i32 @foo(
+// CHECK: callbr {{.*}} "=r,X,{{.*}}"(i8* blockaddress(@foo, %indirect))
+// CHECK-NEXT: to label %asm.fallthrough [label %indirect]
+
+int bar(void) {
+  return foo();
+}
Index: clang/test/Modules/Inputs/asm-goto/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/module.modulemap
@@ -0,0 +1 @@
+module a { header "a.h" export * }
Index: clang/test/Modules/Inputs/asm-goto/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/a.h
@@ -0,0 +1,13 @@
+int foo(void) {
+  int x;
+
+  asm goto(""
+   : "=r"(x)
+   :
+   :
+   : indirect);
+  x = 42;
+
+indirect:
+  return x;
+}
Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -528,7 +528,6 @@
   this->NumInputs = NumInputs;
   this->NumClobbers = NumClobbers;
   this->NumLabels = NumLabels;
-  assert(!(NumOutputs && NumLabels) && "asm goto cannot have outputs");
 
   unsigned NumExprs = NumOutputs + NumInputs + NumLabels;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88195: Remove stale assert.

2020-09-24 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added a comment.

In D88195#2293559 , @nickdesaulniers 
wrote:

> In D88195#2293533 , @void wrote:
>
>> In D88195#2293165 , 
>> @nickdesaulniers wrote:
>>
>>> I'm super confused between the commit message and initial hunk, that seem 
>>> to make sense and probably should go in clang-11 if it's not too late, and 
>>> the additional tests for modules which the commit message does not address. 
>>>  Were these meant to be separate commits, because otherwise it looks like 
>>> you uploaded unrelated stuff.  Are C++20 modules broken with `asm goto`, or 
>>> are you just adding additional test coverage for that?
>>
>> The assert only triggers for modules, so yeah modules are broken with "asm 
>> goto", but only if asserts are enabled.
>
> The assert was removed from AST/Stmt.cpp; it doesn't look module related.  
> Wouldn't it be triggered by ANY `GCCAsmStmt`?  (I have patches that use ASM 
> goto w/ outputs on the kernel, let me try an assertions enabled build).  It's 
> not obvious to me why the method modified would only trigger for modules.

Indeed, I don't trip the assertion in kernel builds using outputs.  Is 
`GCCAsmStmt::setOutputsAndInputsAndClobbers` only called for modules? If so, 
why?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88195

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

[PATCH] D88195: Remove stale assert.

2020-09-24 Thread Bill Wendling via Phabricator via cfe-commits

void added a comment.

In D88195#2293559 , @nickdesaulniers 
wrote:

> In D88195#2293533 , @void wrote:
>
>> In D88195#2293165 , 
>> @nickdesaulniers wrote:
>>
>>> I'm super confused between the commit message and initial hunk, that seem 
>>> to make sense and probably should go in clang-11 if it's not too late, and 
>>> the additional tests for modules which the commit message does not address. 
>>>  Were these meant to be separate commits, because otherwise it looks like 
>>> you uploaded unrelated stuff.  Are C++20 modules broken with `asm goto`, or 
>>> are you just adding additional test coverage for that?
>>
>> The assert only triggers for modules, so yeah modules are broken with "asm 
>> goto", but only if asserts are enabled.
>
> The assert was removed from AST/Stmt.cpp; it doesn't look module related.  
> Wouldn't it be triggered by ANY `GCCAsmStmt`?  (I have patches that use ASM 
> goto w/ outputs on the kernel, let me try an assertions enabled build).  It's 
> not obvious to me why the method modified would only trigger for modules.

Yes, but that particular function is only called during serialization reading. 
It would trigger for any serialization, this just happens to be for modules. 
I'll edit the commit message to be clearer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88195

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

[PATCH] D88195: Remove stale assert.

2020-09-24 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added a comment.

In D88195#2293533 , @void wrote:

> In D88195#2293165 , @nickdesaulniers 
> wrote:
>
>> I'm super confused between the commit message and initial hunk, that seem to 
>> make sense and probably should go in clang-11 if it's not too late, and the 
>> additional tests for modules which the commit message does not address.  
>> Were these meant to be separate commits, because otherwise it looks like you 
>> uploaded unrelated stuff.  Are C++20 modules broken with `asm goto`, or are 
>> you just adding additional test coverage for that?
>
> The assert only triggers for modules, so yeah modules are broken with "asm 
> goto", but only if asserts are enabled.

The assert was removed from AST/Stmt.cpp; it doesn't look module related.  
Wouldn't it be triggered by ANY `GCCAsmStmt`?  (I have patches that use ASM 
goto w/ outputs on the kernel, let me try an assertions enabled build).  It's 
not obvious to me why the method modified would only trigger for modules.

> The official release doesn't have asserts, so I don't know if this counts as 
> a blocker. But it's not a change in semantics, only to remove an assert...

That's fair.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88195

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

[PATCH] D88195: Remove stale assert.

2020-09-24 Thread Bill Wendling via Phabricator via cfe-commits

void added a comment.

In D88195#2293165 , @nickdesaulniers 
wrote:

> I'm super confused between the commit message and initial hunk, that seem to 
> make sense and probably should go in clang-11 if it's not too late, and the 
> additional tests for modules which the commit message does not address.  Were 
> these meant to be separate commits, because otherwise it looks like you 
> uploaded unrelated stuff.  Are C++20 modules broken with `asm goto`, or are 
> you just adding additional test coverage for that?

The assert only triggers for modules, so yeah modules are broken with "asm 
goto", but only if asserts are enabled. The official release doesn't have 
asserts, so I don't know if this counts as a blocker. But it's not a change in 
semantics, only to remove an assert...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88195

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

[PATCH] D88195: Remove stale assert.

2020-09-24 Thread Bill Wendling via Phabricator via cfe-commits

void updated this revision to Diff 294141.
void added a comment.

Fix test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88195

Files:
  clang/lib/AST/Stmt.cpp
  clang/test/Modules/Inputs/asm-goto/a.h
  clang/test/Modules/Inputs/asm-goto/module.modulemap
  clang/test/Modules/asm-goto.c


Index: clang/test/Modules/asm-goto.c
===
--- /dev/null
+++ clang/test/Modules/asm-goto.c
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c -I%S/Inputs/asm-goto 
-emit-module %S/Inputs/asm-goto/module.modulemap -fmodule-name=a -o %t/a.pcm
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c -I%S/Inputs/asm-goto 
-emit-llvm -o - %s -fmodule-file=%t/a.pcm | FileCheck %s
+#include "a.h"
+
+// CHECK-LABEL: define i32 @foo(
+// CHECK: callbr {{.*}} "=r,X,{{.*}}"(i8* blockaddress(@foo, %indirect))
+// CHECK-NEXT: to label %asm.fallthrough [label %indirect]
+
+int bar(void) {
+  return foo();
+}
Index: clang/test/Modules/Inputs/asm-goto/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/module.modulemap
@@ -0,0 +1 @@
+module a { header "a.h" export * }
Index: clang/test/Modules/Inputs/asm-goto/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/a.h
@@ -0,0 +1,13 @@
+int foo(void) {
+  int x;
+
+  asm goto(""
+   : "=r"(x)
+   :
+   :
+   : indirect);
+  x = 42;
+
+indirect:
+  return x;
+}
Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -528,7 +528,6 @@
   this->NumInputs = NumInputs;
   this->NumClobbers = NumClobbers;
   this->NumLabels = NumLabels;
-  assert(!(NumOutputs && NumLabels) && "asm goto cannot have outputs");
 
   unsigned NumExprs = NumOutputs + NumInputs + NumLabels;
 


Index: clang/test/Modules/asm-goto.c
===
--- /dev/null
+++ clang/test/Modules/asm-goto.c
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c -I%S/Inputs/asm-goto -emit-module %S/Inputs/asm-goto/module.modulemap -fmodule-name=a -o %t/a.pcm
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c -I%S/Inputs/asm-goto -emit-llvm -o - %s -fmodule-file=%t/a.pcm | FileCheck %s
+#include "a.h"
+
+// CHECK-LABEL: define i32 @foo(
+// CHECK: callbr {{.*}} "=r,X,{{.*}}"(i8* blockaddress(@foo, %indirect))
+// CHECK-NEXT: to label %asm.fallthrough [label %indirect]
+
+int bar(void) {
+  return foo();
+}
Index: clang/test/Modules/Inputs/asm-goto/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/module.modulemap
@@ -0,0 +1 @@
+module a { header "a.h" export * }
Index: clang/test/Modules/Inputs/asm-goto/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/a.h
@@ -0,0 +1,13 @@
+int foo(void) {
+  int x;
+
+  asm goto(""
+   : "=r"(x)
+   :
+   :
+   : indirect);
+  x = 42;
+
+indirect:
+  return x;
+}
Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -528,7 +528,6 @@
   this->NumInputs = NumInputs;
   this->NumClobbers = NumClobbers;
   this->NumLabels = NumLabels;
-  assert(!(NumOutputs && NumLabels) && "asm goto cannot have outputs");
 
   unsigned NumExprs = NumOutputs + NumInputs + NumLabels;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88195: Remove stale assert.

2020-09-24 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added a comment.

I'm super confused between the commit message and initial hunk, that seem to 
make sense and probably should go in clang-11 if it's not too late, and the 
additional tests for modules which the commit message does not address.  Were 
these meant to be separate commits, because otherwise it looks like you 
uploaded unrelated stuff.  Are C++20 modules broken with `asm goto`, or are you 
just adding additional test coverage for that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88195

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

[PATCH] D88195: Remove stale assert.

2020-09-24 Thread James Y Knight via Phabricator via cfe-commits

jyknight added inline comments.



Comment at: clang/test/Modules/asm-goto.cpp:1
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I%S/Inputs/asm-goto 
-emit-module %S/Inputs/asm-goto/module.modulemap -fmodule-name=a -o %t/a.pcm

void wrote:
> jyknight wrote:
> > New test seems to be failing on the autobuilders (yay pre-submit 
> > autobuilders!!) for 2 reasons:
> > 1. C++ function-name mangling on Windows is different.
> > 2. "$0 = callbr" != "%0 = callbr"
> > 
> Don't know why I did that. Fixed. :)
Still have problem 1, that @"?foo@@YAHXZ"() != @_Z3foov

Probably simplest to just do
extern "C" {
int foo() {...}
}

to avoid name mangling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88195

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

[PATCH] D88195: Remove stale assert.

2020-09-23 Thread Bill Wendling via Phabricator via cfe-commits

void added inline comments.



Comment at: clang/test/Modules/asm-goto.cpp:1
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I%S/Inputs/asm-goto 
-emit-module %S/Inputs/asm-goto/module.modulemap -fmodule-name=a -o %t/a.pcm

jyknight wrote:
> New test seems to be failing on the autobuilders (yay pre-submit 
> autobuilders!!) for 2 reasons:
> 1. C++ function-name mangling on Windows is different.
> 2. "$0 = callbr" != "%0 = callbr"
> 
Don't know why I did that. Fixed. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88195

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

[PATCH] D88195: Remove stale assert.

2020-09-23 Thread Bill Wendling via Phabricator via cfe-commits

void updated this revision to Diff 293938.
void added a comment.

Fix test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88195

Files:
  clang/lib/AST/Stmt.cpp
  clang/test/Modules/Inputs/asm-goto/a.h
  clang/test/Modules/Inputs/asm-goto/module.modulemap
  clang/test/Modules/asm-goto.cpp


Index: clang/test/Modules/asm-goto.cpp
===
--- /dev/null
+++ clang/test/Modules/asm-goto.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I%S/Inputs/asm-goto 
-emit-module %S/Inputs/asm-goto/module.modulemap -fmodule-name=a -o %t/a.pcm
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I%S/Inputs/asm-goto 
-emit-llvm -o - %s -fmodule-file=%t/a.pcm | FileCheck %s
+#include "a.h"
+
+// CHECK-LABEL: define {{.*}} @_Z3foov(
+// CHECK: callbr {{.*}} "=r,X,{{.*}}"(i8* blockaddress(@_Z3foov, %indirect))
+// CHECK-NEXT: to label %asm.fallthrough [label %indirect]
+
+int bar(void) {
+  return foo();
+}
Index: clang/test/Modules/Inputs/asm-goto/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/module.modulemap
@@ -0,0 +1 @@
+module a { header "a.h" export * }
Index: clang/test/Modules/Inputs/asm-goto/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/a.h
@@ -0,0 +1,9 @@
+int foo(void) {
+  int x;
+
+  asm goto("" : "=r"(x) : : : indirect);
+  x = 42;
+
+indirect:
+  return x;
+}
Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -528,7 +528,6 @@
   this->NumInputs = NumInputs;
   this->NumClobbers = NumClobbers;
   this->NumLabels = NumLabels;
-  assert(!(NumOutputs && NumLabels) && "asm goto cannot have outputs");
 
   unsigned NumExprs = NumOutputs + NumInputs + NumLabels;
 


Index: clang/test/Modules/asm-goto.cpp
===
--- /dev/null
+++ clang/test/Modules/asm-goto.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I%S/Inputs/asm-goto -emit-module %S/Inputs/asm-goto/module.modulemap -fmodule-name=a -o %t/a.pcm
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I%S/Inputs/asm-goto -emit-llvm -o - %s -fmodule-file=%t/a.pcm | FileCheck %s
+#include "a.h"
+
+// CHECK-LABEL: define {{.*}} @_Z3foov(
+// CHECK: callbr {{.*}} "=r,X,{{.*}}"(i8* blockaddress(@_Z3foov, %indirect))
+// CHECK-NEXT: to label %asm.fallthrough [label %indirect]
+
+int bar(void) {
+  return foo();
+}
Index: clang/test/Modules/Inputs/asm-goto/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/module.modulemap
@@ -0,0 +1 @@
+module a { header "a.h" export * }
Index: clang/test/Modules/Inputs/asm-goto/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/a.h
@@ -0,0 +1,9 @@
+int foo(void) {
+  int x;
+
+  asm goto("" : "=r"(x) : : : indirect);
+  x = 42;
+
+indirect:
+  return x;
+}
Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -528,7 +528,6 @@
   this->NumInputs = NumInputs;
   this->NumClobbers = NumClobbers;
   this->NumLabels = NumLabels;
-  assert(!(NumOutputs && NumLabels) && "asm goto cannot have outputs");
 
   unsigned NumExprs = NumOutputs + NumInputs + NumLabels;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88195: Remove stale assert.

2020-09-23 Thread James Y Knight via Phabricator via cfe-commits

jyknight added inline comments.



Comment at: clang/test/Modules/Inputs/asm-goto/a.h:4-6
+  asm goto("xor $0, $0\n\t"
+   "test $0, $0\n\t"
+   "jne $l1\n\t"

An empty asm string will suffice for the test.



Comment at: clang/test/Modules/asm-goto.cpp:1
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I%S/Inputs/asm-goto 
-emit-module %S/Inputs/asm-goto/module.modulemap -fmodule-name=a -o %t/a.pcm

New test seems to be failing on the autobuilders (yay pre-submit 
autobuilders!!) for 2 reasons:
1. C++ function-name mangling on Windows is different.
2. "$0 = callbr" != "%0 = callbr"



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88195

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

[PATCH] D88195: Remove stale assert.

2020-09-23 Thread Bill Wendling via Phabricator via cfe-commits

void created this revision.
void added reviewers: nickdesaulniers, jyknight.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
void requested review of this revision.

Remove assert that "asm goto" cannot have outputs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88195

Files:
  clang/lib/AST/Stmt.cpp
  clang/test/Modules/Inputs/asm-goto/a.h
  clang/test/Modules/Inputs/asm-goto/module.modulemap
  clang/test/Modules/asm-goto.cpp


Index: clang/test/Modules/asm-goto.cpp
===
--- /dev/null
+++ clang/test/Modules/asm-goto.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I%S/Inputs/asm-goto 
-emit-module %S/Inputs/asm-goto/module.modulemap -fmodule-name=a -o %t/a.pcm
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I%S/Inputs/asm-goto 
-emit-llvm -o - %s -fmodule-file=%t/a.pcm | FileCheck %s
+#include "a.h"
+
+// CHECK-LABEL: define {{.*}} @_Z3foov(
+// CHECK: $0 = callbr {{.*}} "=r,X,{{.*}}"(i8* blockaddress(@_Z3foov, 
%indirect))
+// CHECK-NEXT: to label %asm.fallthrough [label %indirect]
+
+int bar(void) {
+  return foo();
+}
Index: clang/test/Modules/Inputs/asm-goto/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/module.modulemap
@@ -0,0 +1 @@
+module a { header "a.h" export * }
Index: clang/test/Modules/Inputs/asm-goto/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/a.h
@@ -0,0 +1,12 @@
+int foo(void) {
+  int x;
+
+  asm goto("xor $0, $0\n\t"
+   "test $0, $0\n\t"
+   "jne $l1\n\t"
+   : "=r"(x) : : : indirect);
+  x = 42;
+
+indirect:
+  return x;
+}
Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -528,7 +528,6 @@
   this->NumInputs = NumInputs;
   this->NumClobbers = NumClobbers;
   this->NumLabels = NumLabels;
-  assert(!(NumOutputs && NumLabels) && "asm goto cannot have outputs");
 
   unsigned NumExprs = NumOutputs + NumInputs + NumLabels;
 


Index: clang/test/Modules/asm-goto.cpp
===
--- /dev/null
+++ clang/test/Modules/asm-goto.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I%S/Inputs/asm-goto -emit-module %S/Inputs/asm-goto/module.modulemap -fmodule-name=a -o %t/a.pcm
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I%S/Inputs/asm-goto -emit-llvm -o - %s -fmodule-file=%t/a.pcm | FileCheck %s
+#include "a.h"
+
+// CHECK-LABEL: define {{.*}} @_Z3foov(
+// CHECK: $0 = callbr {{.*}} "=r,X,{{.*}}"(i8* blockaddress(@_Z3foov, %indirect))
+// CHECK-NEXT: to label %asm.fallthrough [label %indirect]
+
+int bar(void) {
+  return foo();
+}
Index: clang/test/Modules/Inputs/asm-goto/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/module.modulemap
@@ -0,0 +1 @@
+module a { header "a.h" export * }
Index: clang/test/Modules/Inputs/asm-goto/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/asm-goto/a.h
@@ -0,0 +1,12 @@
+int foo(void) {
+  int x;
+
+  asm goto("xor $0, $0\n\t"
+   "test $0, $0\n\t"
+   "jne $l1\n\t"
+   : "=r"(x) : : : indirect);
+  x = 42;
+
+indirect:
+  return x;
+}
Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -528,7 +528,6 @@
   this->NumInputs = NumInputs;
   this->NumClobbers = NumClobbers;
   this->NumLabels = NumLabels;
-  assert(!(NumOutputs && NumLabels) && "asm goto cannot have outputs");
 
   unsigned NumExprs = NumOutputs + NumInputs + NumLabels;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88195: Remove stale assert.

[PATCH] D88195: Remove stale assert.

[PATCH] D88195: Remove stale assert.

[PATCH] D88195: Remove stale assert.

[PATCH] D88195: Remove stale assert.

[PATCH] D88195: Remove stale assert.

[PATCH] D88195: Remove stale assert.

[PATCH] D88195: Remove stale assert.

[PATCH] D88195: Remove stale assert.

[PATCH] D88195: Remove stale assert.

[PATCH] D88195: Remove stale assert.

[PATCH] D88195: Remove stale assert.

[PATCH] D88195: Remove stale assert.

[PATCH] D88195: Remove stale assert.

[PATCH] D88195: Remove stale assert.

[PATCH] D88195: Remove stale assert.

16 matches

Site Navigation

Mail list logo

Footer information