Re: [PATCH] D24193: Allow variables with asm labels in naked functions

2016-09-13 Thread Nikola Smiljanić via cfe-commits 
nikola closed this revision.
nikola added a comment.

r281298


https://reviews.llvm.org/D24193



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

Re: [PATCH] D24193: Allow variables with asm labels in naked functions

2016-09-12 Thread Hans Wennborg via cfe-commits 
hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

lgtm

I'm a little bit worried that we've started down a slippery slope of deciding 
what is and isn't allowed in naked functions (for example, some initializers 
could be allowed if we're sure they don't require stack). But I suppose this 
isn't making anything worse, and if it helps compile real-world code, that's 
good.


https://reviews.llvm.org/D24193



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

Re: [PATCH] D24193: Allow variables with asm labels in naked functions

2016-09-12 Thread Nikola Smiljanić via cfe-commits 
nikola updated this revision to Diff 70985.
nikola added a comment.

This should address Hans' comments, as for the code get I have no idea. I was 
hoping someone more knowledgeable would tell me if this made sense or not?


https://reviews.llvm.org/D24193

Files:
  lib/Sema/SemaDecl.cpp
  test/Sema/attr-naked.c

Index: test/Sema/attr-naked.c
===
--- test/Sema/attr-naked.c
+++ test/Sema/attr-naked.c
@@ -48,3 +48,21 @@
"r"(z) // expected-error{{parameter references not allowed in 
naked functions}}
);
 }
+
+__attribute__((naked)) void t10() {  // expected-note{{attribute is here}}
+  int a; // expected-error{{non-ASM statement in naked function is not 
supported}}
+}
+
+__attribute__((naked)) void t11() {  // expected-note{{attribute is here}}
+  register int a asm("eax") = x; // expected-error{{non-ASM statement in naked 
function is not supported}}
+}
+
+__attribute__((naked)) void t12() {  // expected-note{{attribute is here}}
+  register int a asm("eax"), b asm("ebx") = x; // expected-error{{non-ASM 
statement in naked function is not supported}}
+}
+
+__attribute__((naked)) void t13() {
+  register int a asm("eax");
+  register int b asm("ebx"), c asm("ecx");
+}
+
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11792,6 +11792,21 @@
 
 if (FD && FD->hasAttr()) {
   for (const Stmt *S : Body->children()) {
+// Allow local register variables without initializer as they don't
+// require prologue.
+bool RegisterVariables = false;
+if (auto *DS = dyn_cast(S)) {
+  for (const auto *Decl : DS->decls()) {
+if (const auto *Var = dyn_cast(Decl)) {
+  RegisterVariables =
+  Var->hasAttr() && !Var->hasInit();
+  if (!RegisterVariables)
+break;
+}
+  }
+}
+if (RegisterVariables)
+  continue;
 if (!isa(S) && !isa(S)) {
   Diag(S->getLocStart(), diag::err_non_asm_stmt_in_naked_function);
   Diag(FD->getAttr()->getLocation(), diag::note_attribute);


Index: test/Sema/attr-naked.c
===
--- test/Sema/attr-naked.c
+++ test/Sema/attr-naked.c
@@ -48,3 +48,21 @@
"r"(z) // expected-error{{parameter references not allowed in naked functions}}
);
 }
+
+__attribute__((naked)) void t10() {  // expected-note{{attribute is here}}
+  int a; // expected-error{{non-ASM statement in naked function is not supported}}
+}
+
+__attribute__((naked)) void t11() {  // expected-note{{attribute is here}}
+  register int a asm("eax") = x; // expected-error{{non-ASM statement in naked function is not supported}}
+}
+
+__attribute__((naked)) void t12() {  // expected-note{{attribute is here}}
+  register int a asm("eax"), b asm("ebx") = x; // expected-error{{non-ASM statement in naked function is not supported}}
+}
+
+__attribute__((naked)) void t13() {
+  register int a asm("eax");
+  register int b asm("ebx"), c asm("ecx");
+}
+
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11792,6 +11792,21 @@
 
 if (FD && FD->hasAttr()) {
   for (const Stmt *S : Body->children()) {
+// Allow local register variables without initializer as they don't
+// require prologue.
+bool RegisterVariables = false;
+if (auto *DS = dyn_cast(S)) {
+  for (const auto *Decl : DS->decls()) {
+if (const auto *Var = dyn_cast(Decl)) {
+  RegisterVariables =
+  Var->hasAttr() && !Var->hasInit();
+  if (!RegisterVariables)
+break;
+}
+  }
+}
+if (RegisterVariables)
+  continue;
 if (!isa(S) && !isa(S)) {
   Diag(S->getLocStart(), diag::err_non_asm_stmt_in_naked_function);
   Diag(FD->getAttr()->getLocation(), diag::note_attribute);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24193: Allow variables with asm labels in naked functions

2016-09-02 Thread Reid Kleckner via cfe-commits 
rnk added a comment.

Won't the mid-level optimizer turn these register variables into undefs, and 
we'll end up with this kind of IR?

  void f() {
int register var asm ("eax") ;
asm volatile ("add %%eax, %0\n\tret" : : "r"(var));
  }
  ->
  define void @"\01?f@@YAXXZ"() local_unnamed_addr #0 {
  entry:
tail call void asm sideeffect "add %eax, $0\0A\09ret", 
"{eax},~{dirflag},~{fpsr},~{flags}"(i32 undef) #1, !srcloc !2
ret void
  }

I guess it's OK so long as we don't codegen undef to anything, but it could go 
badly.


https://reviews.llvm.org/D24193



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

Re: [PATCH] D24193: Allow variables with asm labels in naked functions

2016-09-02 Thread Hans Wennborg via cfe-commits 
hans added a comment.

I think this is reasonable. Just a few comments:



Comment at: lib/Sema/SemaDecl.cpp:11796
@@ +11795,3 @@
+if (auto *DS = dyn_cast(S)) {
+  if (DS->isSingleDecl()) {
+if (auto *Var = dyn_cast_or_null(DS->getSingleDecl())) {

I think we'll need to check for multiple declarators here:

```
register int x asm("eax"), y asm("ebx");
```

And do we need to check for an initializer? For example, the following should 
not be allowed:

```
register int x asm("eax") = g();
```

It would also be nice to have a comment explaining why these are Ok but not 
other declarations.


https://reviews.llvm.org/D24193



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

[PATCH] D24193: Allow variables with asm labels in naked functions

2016-09-02 Thread Nikola Smiljanić via cfe-commits 
nikola created this revision.
nikola added reviewers: hans, rnk, compnerd.
nikola added a subscriber: cfe-commits.
Herald added a subscriber: aemerson.

I think the current mode is too restrictive, it will emit error for any 
statement inside a naked function. Code I'm trying to compile for ARM declares 
registers as variables to improve readability and passes them as input operands 
to inline assembly.

register uint32_t Something asm("rax");


https://reviews.llvm.org/D24193

Files:
  lib/Sema/SemaDecl.cpp
  test/Sema/attr-naked.c

Index: test/Sema/attr-naked.c
===
--- test/Sema/attr-naked.c
+++ test/Sema/attr-naked.c
@@ -48,3 +48,12 @@
"r"(z) // expected-error{{parameter references not allowed in 
naked functions}}
);
 }
+
+__attribute__((naked)) void t10() {  // expected-note{{attribute is here}}
+  int x; // expected-error{{non-ASM statement in naked function is not 
supported}}
+}
+
+__attribute__((naked)) void t11() {
+  register int x asm("eax");
+}
+
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11792,6 +11792,14 @@
 
 if (FD && FD->hasAttr()) {
   for (const Stmt *S : Body->children()) {
+if (auto *DS = dyn_cast(S)) {
+  if (DS->isSingleDecl()) {
+if (auto *Var = dyn_cast_or_null(DS->getSingleDecl())) {
+  if (Var->hasAttr())
+continue;
+}
+  }
+}
 if (!isa(S) && !isa(S)) {
   Diag(S->getLocStart(), diag::err_non_asm_stmt_in_naked_function);
   Diag(FD->getAttr()->getLocation(), diag::note_attribute);


Index: test/Sema/attr-naked.c
===
--- test/Sema/attr-naked.c
+++ test/Sema/attr-naked.c
@@ -48,3 +48,12 @@
"r"(z) // expected-error{{parameter references not allowed in naked functions}}
);
 }
+
+__attribute__((naked)) void t10() {  // expected-note{{attribute is here}}
+  int x; // expected-error{{non-ASM statement in naked function is not supported}}
+}
+
+__attribute__((naked)) void t11() {
+  register int x asm("eax");
+}
+
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11792,6 +11792,14 @@
 
 if (FD && FD->hasAttr()) {
   for (const Stmt *S : Body->children()) {
+if (auto *DS = dyn_cast(S)) {
+  if (DS->isSingleDecl()) {
+if (auto *Var = dyn_cast_or_null(DS->getSingleDecl())) {
+  if (Var->hasAttr())
+continue;
+}
+  }
+}
 if (!isa(S) && !isa(S)) {
   Diag(S->getLocStart(), diag::err_non_asm_stmt_in_naked_function);
   Diag(FD->getAttr()->getLocation(), diag::note_attribute);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits