[PATCH] D72221: Support function attribute patchable_function_entry

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa44c434b68e5: Support function attribute 
patchable_function_entry (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/patchable-function-entry.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/patchable-function-entry-attr.c
  clang/test/Sema/patchable-function-entry-attr.cpp

Index: clang/test/Sema/patchable-function-entry-attr.cpp
===
--- /dev/null
+++ clang/test/Sema/patchable-function-entry-attr.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple aarch64 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple i386 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple x86_64 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple ppc64le -fsyntax-only -verify %s
+
+// silence-no-diagnostics
+
+// expected-warning@+1 {{unknown attribute 'patchable_function_entry' ignored}}
+[[gnu::patchable_function_entry(0)]] void f();
Index: clang/test/Sema/patchable-function-entry-attr.c
===
--- /dev/null
+++ clang/test/Sema/patchable-function-entry-attr.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple aarch64 -fsyntax-only -verify %s
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes at least 1 argument}}
+__attribute__((patchable_function_entry)) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes no more than 2 arguments}}
+__attribute__((patchable_function_entry(0, 0, 0))) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute requires a non-negative integral compile time constant expression}}
+__attribute__((patchable_function_entry(-1))) void f();
+
+int i;
+// expected-error@+1 {{'patchable_function_entry' attribute requires parameter 0 to be an integer constant}}
+__attribute__((patchable_function_entry(i))) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute requires integer constant between 0 and 0 inclusive}}
+__attribute__((patchable_function_entry(1, 1))) void f();
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -128,6 +128,7 @@
 // CHECK-NEXT: Owner (SubjectMatchRule_record_not_is_union)
 // CHECK-NEXT: ParamTypestate (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: PassObjectSize (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: PatchableFunctionEntry (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: Pointer (SubjectMatchRule_record_not_is_union)
 // CHECK-NEXT: ReleaseHandle (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: RenderScriptKernel (SubjectMatchRule_function)
Index: clang/test/CodeGen/patchable-function-entry.c
===
--- /dev/null
+++ clang/test/CodeGen/patchable-function-entry.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple aarch64 -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: define void @f0() #0
+__attribute__((patchable_function_entry(0))) void f0() {}
+
+// CHECK: define void @f00() #0
+__attribute__((patchable_function_entry(0, 0))) void f00() {}
+
+// CHECK: define void @f2() #1
+__attribute__((patchable_function_entry(2))) void f2() {}
+
+// CHECK: define void @f20() #1
+__attribute__((patchable_function_entry(2, 0))) void f20() {}
+
+// CHECK: define void @f20decl() #1
+__attribute__((patchable_function_entry(2, 0))) void f20decl();
+void f20decl() {}
+
+/// M in patchable_function_entry(N,M) is currently ignored.
+// CHECK: attributes #0 = { {{.*}} "patchable-function-entry"="0"
+// CHECK: attributes #1 = { {{.*}} "patchable-function-entry"="2"
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4915,6 +4915,25 @@
  XRayLogArgsAttr(S.Context, AL, ArgCount.getSourceIndex()));
 }
 
+static void handlePatchableFunctionEntryAttr(Sema , Decl *D,
+ const ParsedAttr ) {
+  uint32_t Count = 0, Offset = 0;
+  if (!checkUInt32Argument(S, AL, AL.getArgAsExpr(0), Count, 0, true))
+return;
+  if (AL.getNumArgs() == 2) {
+Expr *Arg = AL.getArgAsExpr(1);
+if (!checkUInt32Argument(S, AL, Arg, Offset, 1, true))
+  return;
+if (Offset) {
+  S.Diag(getAttrLoc(AL), diag::err_attribute_argument_out_of_range)
+  <<  << 0 << 

[PATCH] D72221: Support function attribute patchable_function_entry

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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

[PATCH] D72221: Support function attribute patchable_function_entry

[Nick Desaulniers via Phabricator via cfe-commits]

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

Thanks for all of the work that went into this. Looks like review comments have 
all been addressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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

[PATCH] D72221: Support function attribute patchable_function_entry

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 61271 tests passed, 1 failed 
and 736 were skipped.

  failed: Clang.CodeGen/patchable-function-entry.c

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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

[PATCH] D72221: Support function attribute patchable_function_entry

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 236468.
MaskRay marked 13 inline comments as done.
MaskRay added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/patchable-function-entry.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/patchable-function-entry-attr.c
  clang/test/Sema/patchable-function-entry-attr.cpp

Index: clang/test/Sema/patchable-function-entry-attr.cpp
===
--- /dev/null
+++ clang/test/Sema/patchable-function-entry-attr.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple aarch64 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple i386 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple x86_64 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple ppc64le -fsyntax-only -verify %s
+
+// silence-no-diagnostics
+
+// expected-warning@+1 {{unknown attribute 'patchable_function_entry' ignored}}
+[[gnu::patchable_function_entry(0)]] void f();
Index: clang/test/Sema/patchable-function-entry-attr.c
===
--- /dev/null
+++ clang/test/Sema/patchable-function-entry-attr.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple aarch64 -fsyntax-only -verify %s
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes at least 1 argument}}
+__attribute__((patchable_function_entry)) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes no more than 2 arguments}}
+__attribute__((patchable_function_entry(0, 0, 0))) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute requires a non-negative integral compile time constant expression}}
+__attribute__((patchable_function_entry(-1))) void f();
+
+int i;
+// expected-error@+1 {{'patchable_function_entry' attribute requires parameter 0 to be an integer constant}}
+__attribute__((patchable_function_entry(i))) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute requires integer constant between 0 and 0 inclusive}}
+__attribute__((patchable_function_entry(1, 1))) void f();
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -128,6 +128,7 @@
 // CHECK-NEXT: Owner (SubjectMatchRule_record_not_is_union)
 // CHECK-NEXT: ParamTypestate (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: PassObjectSize (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: PatchableFunctionEntry (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: Pointer (SubjectMatchRule_record_not_is_union)
 // CHECK-NEXT: ReleaseHandle (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: RenderScriptKernel (SubjectMatchRule_function)
Index: clang/test/CodeGen/patchable-function-entry.c
===
--- /dev/null
+++ clang/test/CodeGen/patchable-function-entry.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple aarch64 -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: define void @f0() #0
+__attribute__((patchable_function_entry(0))) void f0() {}
+
+// CHECK: define void @f00() #0
+__attribute__((patchable_function_entry(0, 0))) void f00() {}
+
+// CHECK: define void @f2() #1
+__attribute__((patchable_function_entry(2))) void f2() {}
+
+// CHECK: define void @f20() #1
+__attribute__((patchable_function_entry(2, 0))) void f20() {}
+
+// CHECK: define void @f20decl() #1
+__attribute__((patchable_function_entry(2, 0))) void f20decl();
+void f20decl() {}
+
+/// M in patchable_function_entry(N,M) is currently ignored.
+// CHECK: attributes #0 = { {{.*}} "patchable-function-entry"="0"
+// CHECK: attributes #1 = { {{.*}} "patchable-function-entry"="2"
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4917,6 +4917,25 @@
  XRayLogArgsAttr(S.Context, AL, ArgCount.getSourceIndex()));
 }
 
+static void handlePatchableFunctionEntryAttr(Sema , Decl *D,
+ const ParsedAttr ) {
+  uint32_t Count = 0, Offset = 0;
+  if (!checkUInt32Argument(S, AL, AL.getArgAsExpr(0), Count, 0, true))
+return;
+  if (AL.getNumArgs() == 2) {
+Expr *Arg = AL.getArgAsExpr(1);
+if (!checkUInt32Argument(S, AL, Arg, Offset, 1, true))
+  return;
+if (Offset) {
+  S.Diag(getAttrLoc(AL), diag::err_attribute_argument_out_of_range)
+  <<  << 0 << 0 << Arg->getBeginLoc();
+  return;
+  

[PATCH] D72221: Support function attribute patchable_function_entry

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:686
 
+def PatchableFunctionEntry : InheritableAttr {
+  let Spellings = [GCC<"patchable_function_entry">];

aaron.ballman wrote:
> Should this be inheriting from `TargetSpecificAttr` as well given that the 
> attribute is only supported on some architectures?
Thanks for the suggestion. Didn't know TargetSpecificAttr.



Comment at: clang/include/clang/Basic/AttrDocs.td:3995
+``__attribute__((patchable_function_entry(N,M)))`` is used to generate M NOPs
+before the function entry and N-M NOPs after the function entry. This 
attributes
+takes precedence over command line option ``-fpatchable-function-entry=N,M``.

ostannard wrote:
> Grammar nits:
> s/attributes/attribute/
> over _the_ command line option
Thanks. (I am a non-native English speaker and any advice will be highly 
appreciated.)

(So, I believe GCC's documentation misses _the_)



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:826
+  "patchable-function-entry",
+  (Twine(Attr->getSize()) + "," + Twine(Attr->getStart())).str());
   }

nickdesaulniers wrote:
> ostannard wrote:
> > I think using two function attributes here would be better, to avoid 
> > needing to parse this again later.
> In that case, it would not make sense to have start without a size, and thus 
> should be checked for in the verification.
Agreed. Updated D72215 (semantics of the IR function attribute 
"patchable-function-entry") as well.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4925
+  T.getArch() != llvm::Triple::x86_64) {
+S.Diag(getAttrLoc(AL), diag::err_attribute_unsupported) << AL;
+return;

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > Why is the target arch also checked in 
> > `clang/lib/Driver/ToolChains/Clang.cpp` in https://reviews.llvm.org/D7?
> Ah, https://reviews.llvm.org/D72221 is checking the `__attribute__` syntax, 
> https://reviews.llvm.org/D7 is checking the command line `-f` syntax.
Yes. D72221 is for the Clang function attribute while D7 is for 
`-fpatchable-function-entry=`.





Comment at: clang/test/CodeGen/patchable-function-entry.c:20
+// CHECK: attributes #0 = { {{.*}} "patchable-function-entry"="0,0"
+// CHECK: attributes #1 = { {{.*}} "patchable-function-entry"="2,0"

aaron.ballman wrote:
> Can you also add a test verifying that this attribute works with 
> redeclarations as well? Something like:
> ```
> [[gnu::patchable_function_entry(12, 4)]] void func(void);
> 
> void func(void) { // Definition should have the noop sled
> }
> ```
```
__attribute__((patchable_function_entry(2,0))) void f20decl();
__attribute__((noinline)) void f20decl() {}
```

Checked this case. I'll delete `__attribute__((noinline))` since it does not 
seem to be clear what it intends to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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

[PATCH] D72221: Support function attribute patchable_function_entry

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:686
 
+def PatchableFunctionEntry : InheritableAttr {
+  let Spellings = [GCC<"patchable_function_entry">];

Should this be inheriting from `TargetSpecificAttr` as well given that the 
attribute is only supported on some architectures?



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4923
+  const llvm::Triple  = S.Context.getTargetInfo().getTriple();
+  if (!T.isAArch64() && T.getArch() != llvm::Triple::x86 &&
+  T.getArch() != llvm::Triple::x86_64) {

If you make the attribute target-specific, this code can be removed.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4930
+  uint32_t Size = 0, Start = 0;
+  if (AL.getNumArgs()) {
+if (!checkUInt32Argument(S, AL, AL.getArgAsExpr(0), Size, 0, true))

Should not be a need to check this -- it will always have at least one argument 
because the common attribute handling code will diagnose otherwise given the 
subject list.



Comment at: clang/test/CodeGen/patchable-function-entry.c:20
+// CHECK: attributes #0 = { {{.*}} "patchable-function-entry"="0,0"
+// CHECK: attributes #1 = { {{.*}} "patchable-function-entry"="2,0"

Can you also add a test verifying that this attribute works with redeclarations 
as well? Something like:
```
[[gnu::patchable_function_entry(12, 4)]] void func(void);

void func(void) { // Definition should have the noop sled
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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

[PATCH] D72221: Support function attribute patchable_function_entry

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:826
+  "patchable-function-entry",
+  (Twine(Attr->getSize()) + "," + Twine(Attr->getStart())).str());
   }

ostannard wrote:
> I think using two function attributes here would be better, to avoid needing 
> to parse this again later.
In that case, it would not make sense to have start without a size, and thus 
should be checked for in the verification.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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

[PATCH] D72221: Support function attribute patchable_function_entry

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers marked an inline comment as done.
nickdesaulniers added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4925
+  T.getArch() != llvm::Triple::x86_64) {
+S.Diag(getAttrLoc(AL), diag::err_attribute_unsupported) << AL;
+return;

nickdesaulniers wrote:
> Why is the target arch also checked in 
> `clang/lib/Driver/ToolChains/Clang.cpp` in https://reviews.llvm.org/D7?
Ah, https://reviews.llvm.org/D72221 is checking the `__attribute__` syntax, 
https://reviews.llvm.org/D7 is checking the command line `-f` syntax.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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

[PATCH] D72221: Support function attribute patchable_function_entry

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4925
+  T.getArch() != llvm::Triple::x86_64) {
+S.Diag(getAttrLoc(AL), diag::err_attribute_unsupported) << AL;
+return;

Why is the target arch also checked in `clang/lib/Driver/ToolChains/Clang.cpp` 
in https://reviews.llvm.org/D7?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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

[PATCH] D72221: Support function attribute patchable_function_entry

[Oliver Stannard (Linaro) via Phabricator via cfe-commits]

ostannard added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:3995
+``__attribute__((patchable_function_entry(N,M)))`` is used to generate M NOPs
+before the function entry and N-M NOPs after the function entry. This 
attributes
+takes precedence over command line option ``-fpatchable-function-entry=N,M``.

Grammar nits:
s/attributes/attribute/
over _the_ command line option



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:826
+  "patchable-function-entry",
+  (Twine(Attr->getSize()) + "," + Twine(Attr->getStart())).str());
   }

I think using two function attributes here would be better, to avoid needing to 
parse this again later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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

[PATCH] D72221: Support function attribute patchable_function_entry

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61253 tests passed, 0 failed 
and 736 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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

[PATCH] D72221: Support function attribute patchable_function_entry

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61253 tests passed, 0 failed 
and 736 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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

[PATCH] D72221: Support function attribute patchable_function_entry

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 236225.
MaskRay added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/patchable-function-entry.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/patchable-function-entry-attr.c
  clang/test/Sema/patchable-function-entry-attr.cpp

Index: clang/test/Sema/patchable-function-entry-attr.cpp
===
--- /dev/null
+++ clang/test/Sema/patchable-function-entry-attr.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple aarch64 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple i386 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple x86_64 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple ppc64le -fsyntax-only -verify %s
+
+// silence-no-diagnostics
+
+// expected-error@+1 {{'patchable_function_entry' attribute is not supported for this target}}
+[[gnu::patchable_function_entry(0)]] void f();
Index: clang/test/Sema/patchable-function-entry-attr.c
===
--- /dev/null
+++ clang/test/Sema/patchable-function-entry-attr.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple aarch64 -fsyntax-only -verify %s
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes at least 1 argument}}
+__attribute__((patchable_function_entry)) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes no more than 2 arguments}}
+__attribute__((patchable_function_entry(0, 0, 0))) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute requires a non-negative integral compile time constant expression}}
+__attribute__((patchable_function_entry(-1))) void f();
+
+int i;
+// expected-error@+1 {{'patchable_function_entry' attribute requires parameter 0 to be an integer constant}}
+__attribute__((patchable_function_entry(i))) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute requires integer constant between 0 and 0 inclusive}}
+__attribute__((patchable_function_entry(1, 1))) void f();
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -128,6 +128,7 @@
 // CHECK-NEXT: Owner (SubjectMatchRule_record_not_is_union)
 // CHECK-NEXT: ParamTypestate (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: PassObjectSize (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: PatchableFunctionEntry (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: Pointer (SubjectMatchRule_record_not_is_union)
 // CHECK-NEXT: ReleaseHandle (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: RenderScriptKernel (SubjectMatchRule_function)
Index: clang/test/CodeGen/patchable-function-entry.c
===
--- /dev/null
+++ clang/test/CodeGen/patchable-function-entry.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple aarch64 -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: define void @f0() #0
+__attribute__((patchable_function_entry(0))) void f0() {}
+
+// CHECK: define void @f00() #0
+__attribute__((patchable_function_entry(0, 0))) void f00() {}
+
+// CHECK: define void @f2() #1
+__attribute__((patchable_function_entry(2))) void f2() {}
+
+// CHECK: define void @f20() #1
+__attribute__((patchable_function_entry(2, 0))) void f20() {}
+
+// CHECK: define void @f20decl() #1
+__attribute__((patchable_function_entry(2, 0))) void f20decl();
+__attribute__((noinline)) void f20decl() {}
+
+// CHECK: attributes #0 = { {{.*}} "patchable-function-entry"="0,0"
+// CHECK: attributes #1 = { {{.*}} "patchable-function-entry"="2,0"
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4917,6 +4917,34 @@
  XRayLogArgsAttr(S.Context, AL, ArgCount.getSourceIndex()));
 }
 
+static void handlePatchableFunctionEntryAttr(Sema , Decl *D,
+ const ParsedAttr ) {
+  const llvm::Triple  = S.Context.getTargetInfo().getTriple();
+  if (!T.isAArch64() && T.getArch() != llvm::Triple::x86 &&
+  T.getArch() != llvm::Triple::x86_64) {
+S.Diag(getAttrLoc(AL), diag::err_attribute_unsupported) << AL;
+return;
+  }
+
+  uint32_t Size = 0, Start = 0;
+  if (AL.getNumArgs()) {
+if (!checkUInt32Argument(S, AL, AL.getArgAsExpr(0), Size, 0, true))
+  return;
+if (AL.getNumArgs() == 2) {
+  Expr *Arg = AL.getArgAsExpr(1);
+  if 

[PATCH] D72221: Support function attribute patchable_function_entry

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61253 tests passed, 0 failed 
and 736 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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

[PATCH] D72221: Support function attribute patchable_function_entry

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 236221.
MaskRay added a comment.

Move a line from D7  to D72221 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/patchable-function-entry.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/patchable-function-entry-attr.c
  clang/test/Sema/patchable-function-entry-attr.cpp

Index: clang/test/Sema/patchable-function-entry-attr.cpp
===
--- /dev/null
+++ clang/test/Sema/patchable-function-entry-attr.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple aarch64 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple i386 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple x86_64 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple ppc64le -fsyntax-only -verify %s
+
+// silence-no-diagnostics
+
+// expected-error@+1 {{'patchable_function_entry' attribute is not supported for this target}}
+[[gnu::patchable_function_entry(0)]] void f();
Index: clang/test/Sema/patchable-function-entry-attr.c
===
--- /dev/null
+++ clang/test/Sema/patchable-function-entry-attr.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple aarch64 -fsyntax-only -verify %s
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes at least 1 argument}}
+__attribute__((patchable_function_entry)) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes no more than 2 arguments}}
+__attribute__((patchable_function_entry(0,0,0))) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute requires a non-negative integral compile time constant expression}}
+__attribute__((patchable_function_entry(-1))) void f();
+
+int i;
+// expected-error@+1 {{'patchable_function_entry' attribute requires parameter 0 to be an integer constant}}
+__attribute__((patchable_function_entry(i))) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute requires integer constant between 0 and 0 inclusive}}
+__attribute__((patchable_function_entry(1,1))) void f();
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -128,6 +128,7 @@
 // CHECK-NEXT: Owner (SubjectMatchRule_record_not_is_union)
 // CHECK-NEXT: ParamTypestate (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: PassObjectSize (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: PatchableFunctionEntry (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: Pointer (SubjectMatchRule_record_not_is_union)
 // CHECK-NEXT: ReleaseHandle (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: RenderScriptKernel (SubjectMatchRule_function)
Index: clang/test/CodeGen/patchable-function-entry.c
===
--- /dev/null
+++ clang/test/CodeGen/patchable-function-entry.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple aarch64 -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: define void @f0() #0
+__attribute__((patchable_function_entry(0))) void f0() {}
+
+// CHECK: define void @f00() #0
+__attribute__((patchable_function_entry(0,0))) void f00() {}
+
+// CHECK: define void @f2() #1
+__attribute__((patchable_function_entry(2))) void f2() {}
+
+// CHECK: define void @f20() #1
+__attribute__((patchable_function_entry(2,0))) void f20() {}
+
+// CHECK: define void @f20decl() #1
+__attribute__((patchable_function_entry(2,0))) void f20decl();
+__attribute__((noinline)) void f20decl() {}
+
+// CHECK: attributes #0 = { {{.*}} "patchable-function-entry"="0,0"
+// CHECK: attributes #1 = { {{.*}} "patchable-function-entry"="2,0"
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4917,6 +4917,34 @@
  XRayLogArgsAttr(S.Context, AL, ArgCount.getSourceIndex()));
 }
 
+static void handlePatchableFunctionEntryAttr(Sema , Decl *D,
+ const ParsedAttr ) {
+  const llvm::Triple  = S.Context.getTargetInfo().getTriple();
+  if (!T.isAArch64() && T.getArch() != llvm::Triple::x86 &&
+  T.getArch() != llvm::Triple::x86_64) {
+S.Diag(getAttrLoc(AL), diag::err_attribute_unsupported) << AL;
+return;
+  }
+
+  uint32_t Size = 0, Start = 0;
+  if (AL.getNumArgs()) {
+if (!checkUInt32Argument(S, AL, AL.getArgAsExpr(0), Size, 0, true))
+  return;
+ 

[PATCH] D72221: Support function attribute patchable_function_entry

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: dberris, kristof.beyls, nickdesaulniers, rnk, rsmith, 
void.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
MaskRay added a parent revision: D72220: [X86] Support function attribute 
"patchable-function-entry".
MaskRay added a child revision: D7: [Driver][CodeGen] Add 
-fpatchable-function-entry=N[,M].

This feature is generic. Make it applicable for AArch64 and X86 because
the backend has only implemented NOP insertion for AArch64 and X86.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72221

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/patchable-function-entry.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/patchable-function-entry-attr.c
  clang/test/Sema/patchable-function-entry-attr.cpp

Index: clang/test/Sema/patchable-function-entry-attr.cpp
===
--- /dev/null
+++ clang/test/Sema/patchable-function-entry-attr.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple aarch64 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple x86_64 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple ppc64le -fsyntax-only -verify %s
+
+// silence-no-diagnostics
+
+// expected-error@+1 {{'patchable_function_entry' attribute is not supported for this target}}
+[[gnu::patchable_function_entry(0)]] void f();
Index: clang/test/Sema/patchable-function-entry-attr.c
===
--- /dev/null
+++ clang/test/Sema/patchable-function-entry-attr.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple aarch64 -fsyntax-only -verify %s
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes at least 1 argument}}
+__attribute__((patchable_function_entry)) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes no more than 2 arguments}}
+__attribute__((patchable_function_entry(0,0,0))) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute requires a non-negative integral compile time constant expression}}
+__attribute__((patchable_function_entry(-1))) void f();
+
+int i;
+// expected-error@+1 {{'patchable_function_entry' attribute requires parameter 0 to be an integer constant}}
+__attribute__((patchable_function_entry(i))) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute requires integer constant between 0 and 0 inclusive}}
+__attribute__((patchable_function_entry(1,1))) void f();
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -128,6 +128,7 @@
 // CHECK-NEXT: Owner (SubjectMatchRule_record_not_is_union)
 // CHECK-NEXT: ParamTypestate (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: PassObjectSize (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: PatchableFunctionEntry (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: Pointer (SubjectMatchRule_record_not_is_union)
 // CHECK-NEXT: ReleaseHandle (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: RenderScriptKernel (SubjectMatchRule_function)
Index: clang/test/CodeGen/patchable-function-entry.c
===
--- /dev/null
+++ clang/test/CodeGen/patchable-function-entry.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple aarch64 -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: define void @f0() #0
+__attribute__((patchable_function_entry(0))) void f0() {}
+
+// CHECK: define void @f00() #0
+__attribute__((patchable_function_entry(0,0))) void f00() {}
+
+// CHECK: define void @f2() #1
+__attribute__((patchable_function_entry(2))) void f2() {}
+
+// CHECK: define void @f20() #1
+__attribute__((patchable_function_entry(2,0))) void f20() {}
+
+// CHECK: define void @f20decl() #1
+__attribute__((patchable_function_entry(2,0))) void f20decl();
+__attribute__((noinline)) void f20decl() {}
+
+// CHECK: attributes #0 = { {{.*}} "patchable-function-entry"="0,0"
+// CHECK: attributes #1 = { {{.*}} "patchable-function-entry"="2,0"
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4917,6 +4917,34 @@
  XRayLogArgsAttr(S.Context, AL, ArgCount.getSourceIndex()));
 }
 
+static void handlePatchableFunctionEntryAttr(Sema , Decl *D,
+ const ParsedAttr ) {
+  const llvm::Triple  = S.Context.getTargetInfo().getTriple();
+  if (!T.isAArch64() && T.getArch() != llvm::Triple::x86 &&
+  T.getArch() != llvm::Triple::x86_64) {
+