This revision was automatically updated to reflect the committed changes.
Closed by commit rL278050: [Attr] Add support for the `ms_hook_prologue`
attribute. (authored by cdavis).
Changed prior to commit:
https://reviews.llvm.org/D19909?vs=66729&id=67224#toc
Repository:
rL LLVM
https://revi
cdavis5x marked an inline comment as done.
Comment at: include/clang/Basic/AttrDocs.td:560
@@ +559,3 @@
+
+This attribute cannot be used in conjunction with the ``naked``,
+``always_inline``, or ``__forceinline`` attributes.
Done.
https://reviews.llvm.org/D19909
cdavis5x updated this revision to Diff 66729.
cdavis5x added a comment.
Update for merge conflicts.
- Add a blurb to the docs advising against using this attribute with
`__forceinline`, `always_inline`, or `naked`.
https://reviews.llvm.org/D19909
Files:
include/clang/Basic/Attr.td
include
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM (when Sanjoy's patch goes in), with two minor nits.
Can you also add a test that the attribute fails on one of the target
architectures Windows does not support?
=
cdavis5x updated this revision to Diff 57853.
cdavis5x marked an inline comment as done.
cdavis5x added a comment.
- Add Sema tests for the `ms_hook_prologue` attribute.
- Disallow `ms_hook_prologue` on architectures that Windows doesn't support.
- Disallow `ms_hook_prologue` with the `naked` and
cdavis5x added a comment.
For now, I've disallowed it with `naked` and `always_inline`/`__forceinline`
attributes. Do any other attributes affect prologue generation in a way that
might interfere with `ms_hook_prologue`? AFAICT, GCC only disallows
`ms_hook_prologue` on a) nested functions and b
aaron.ballman added a comment.
The patch is missing semantic test cases. I would like to see tests of the
attribute being accepted, as well as rejected because the subject was
incorrect, args were provided, etc.
Also, I believe there are attributes which this attribute should not be
combined w
cdavis5x marked an inline comment as done.
Comment at: include/clang/Basic/Attr.td:2032
@@ -2031,1 +2031,3 @@
+def MSHookPrologue : InheritableAttr {
+ let Spellings = [GCC<"ms_hook_prologue">];
aaron.ballman wrote:
> Does this attribute appertain to all target
cdavis5x updated this revision to Diff 57389.
cdavis5x added a comment.
- Use Sanjoy's `patchable-function` attribute.
- Add documentation for this new attribute.
http://reviews.llvm.org/D19909
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
lib/CodeGen/TargetInfo.cpp
aaron.ballman added a comment.
Generally, I think this should make use of the patchable function functionality
from http://reviews.llvm.org/D19046 when lowering, but I do have some comments
on the attribute itself as well.
Comment at: include/clang/Basic/Attr.td:2032
@@ -2031,
cdavis5x created this revision.
cdavis5x added reviewers: aaron.ballman, rnk.
cdavis5x added a subscriber: cfe-commits.
cdavis5x added a dependency: D19908: [X86] Support the "ms-hotpatch" attribute..
Based on a patch by Michael Mueller.
This attribute specifies that a function can be hooked or p
11 matches
Mail list logo
