Re: [PATCH] D15399: MS inline ASM: mark the function noinline if the asm has labels (PR23715)

[Hans Wennborg via cfe-commits]

hans added inline comments.


Comment at: lib/CodeGen/CGStmt.cpp:2005
@@ -1998,3 +2004,3 @@
  /* IsAlignStack */ false, AsmDialect);
   llvm::CallInst *Result = Builder.CreateCall(IA, Args);
   Result->addAttribute(llvm::AttributeSet::FunctionIndex,

majnemer wrote:
> rnk wrote:
> > If this inline asm has labels, we should also add the noduplicate attribute 
> > to the callsite. That will prevent CFG transforms like tail duplication 
> > from duplicating it.
> Actually, isn't noduplicate sufficient?  I'm having trouble seeing where 
> noinline inhibits problematic transforms over noduplicate.
You're right. I'll just do that.


Comment at: test/CodeGen/ms-inline-asm.c:536
@@ -535,3 +535,3 @@
   }
-  // CHECK-LABEL: define void @label1
+  // CHECK: define void @label1() [[ATTR1:#[0-9]+]] {
   // CHECK: call void asm sideeffect inteldialect 
"{{.*}}__MSASMLABEL_.1__label:\0A\09jmp {{.*}}__MSASMLABEL_.1__label", 
"~{dirflag},~{fpsr},~{flags}"()

rsmith wrote:
> Why is this not a CHECK-LABEL any more?
FileCheck got upset:
error: found 'CHECK-LABEL:' with variable definition or use


http://reviews.llvm.org/D15399



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

Re: [PATCH] D15399: MS inline ASM: mark the function noinline if the asm has labels (PR23715)

[Richard Smith via cfe-commits]

rsmith added a subscriber: rsmith.


Comment at: test/CodeGen/ms-inline-asm.c:536
@@ -535,3 +535,3 @@
   }
-  // CHECK-LABEL: define void @label1
+  // CHECK: define void @label1() [[ATTR1:#[0-9]+]] {
   // CHECK: call void asm sideeffect inteldialect 
"{{.*}}__MSASMLABEL_.1__label:\0A\09jmp {{.*}}__MSASMLABEL_.1__label", 
"~{dirflag},~{fpsr},~{flags}"()

Why is this not a CHECK-LABEL any more?


http://reviews.llvm.org/D15399



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

Re: [PATCH] D15399: MS inline ASM: mark the function noinline if the asm has labels (PR23715)

[Hans Wennborg via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL255201: Mark MS inline ASM 'nodplicate' it it has labels 
(PR23715) (authored by hans).

Changed prior to commit:
  http://reviews.llvm.org/D15399?vs=42353=42365#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D15399

Files:
  cfe/trunk/lib/CodeGen/CGStmt.cpp
  cfe/trunk/test/CodeGen/ms-inline-asm.c

Index: cfe/trunk/lib/CodeGen/CGStmt.cpp
===
--- cfe/trunk/lib/CodeGen/CGStmt.cpp
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp
@@ -1999,6 +1999,15 @@
   Result->addAttribute(llvm::AttributeSet::FunctionIndex,
llvm::Attribute::NoUnwind);
 
+  if (isa()) {
+// If the assembly contains any labels, mark the call noduplicate to 
prevent
+// defining the same ASM label twice (PR23715). This is pretty hacky, but 
it
+// works.
+if (AsmString.find("__MSASMLABEL_") != std::string::npos)
+  Result->addAttribute(llvm::AttributeSet::FunctionIndex,
+   llvm::Attribute::NoDuplicate);
+  }
+
   // Attach readnone and readonly attributes.
   if (!HasSideEffect) {
 if (ReadNone)
Index: cfe/trunk/test/CodeGen/ms-inline-asm.c
===
--- cfe/trunk/test/CodeGen/ms-inline-asm.c
+++ cfe/trunk/test/CodeGen/ms-inline-asm.c
@@ -533,8 +533,8 @@
 label:
 jmp label
   }
-  // CHECK-LABEL: define void @label1
-  // CHECK: call void asm sideeffect inteldialect 
"{{.*}}__MSASMLABEL_.1__label:\0A\09jmp {{.*}}__MSASMLABEL_.1__label", 
"~{dirflag},~{fpsr},~{flags}"()
+  // CHECK-LABEL: define void @label1()
+  // CHECK: call void asm sideeffect inteldialect 
"{{.*}}__MSASMLABEL_.1__label:\0A\09jmp {{.*}}__MSASMLABEL_.1__label", 
"~{dirflag},~{fpsr},~{flags}"() [[ATTR1:#[0-9]+]]
 }
 
 void label2() {
@@ -581,3 +581,6 @@
 }
 // CHECK-LABEL: define i32 @test_indirect_field(
 // CHECK: call i32 asm sideeffect inteldialect "mov eax, dword ptr $1",
+
+// MS ASM containing labels must not be duplicated (PR23715).
+// CHECK: attributes [[ATTR1]] = { {{.*}}noduplicate{{.*}} }


Index: cfe/trunk/lib/CodeGen/CGStmt.cpp
===
--- cfe/trunk/lib/CodeGen/CGStmt.cpp
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp
@@ -1999,6 +1999,15 @@
   Result->addAttribute(llvm::AttributeSet::FunctionIndex,
llvm::Attribute::NoUnwind);
 
+  if (isa()) {
+// If the assembly contains any labels, mark the call noduplicate to prevent
+// defining the same ASM label twice (PR23715). This is pretty hacky, but it
+// works.
+if (AsmString.find("__MSASMLABEL_") != std::string::npos)
+  Result->addAttribute(llvm::AttributeSet::FunctionIndex,
+   llvm::Attribute::NoDuplicate);
+  }
+
   // Attach readnone and readonly attributes.
   if (!HasSideEffect) {
 if (ReadNone)
Index: cfe/trunk/test/CodeGen/ms-inline-asm.c
===
--- cfe/trunk/test/CodeGen/ms-inline-asm.c
+++ cfe/trunk/test/CodeGen/ms-inline-asm.c
@@ -533,8 +533,8 @@
 label:
 jmp label
   }
-  // CHECK-LABEL: define void @label1
-  // CHECK: call void asm sideeffect inteldialect "{{.*}}__MSASMLABEL_.1__label:\0A\09jmp {{.*}}__MSASMLABEL_.1__label", "~{dirflag},~{fpsr},~{flags}"()
+  // CHECK-LABEL: define void @label1()
+  // CHECK: call void asm sideeffect inteldialect "{{.*}}__MSASMLABEL_.1__label:\0A\09jmp {{.*}}__MSASMLABEL_.1__label", "~{dirflag},~{fpsr},~{flags}"() [[ATTR1:#[0-9]+]]
 }
 
 void label2() {
@@ -581,3 +581,6 @@
 }
 // CHECK-LABEL: define i32 @test_indirect_field(
 // CHECK: call i32 asm sideeffect inteldialect "mov eax, dword ptr $1",
+
+// MS ASM containing labels must not be duplicated (PR23715).
+// CHECK: attributes [[ATTR1]] = { {{.*}}noduplicate{{.*}} }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits