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 &T = 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
