[PATCH] D110869: [X86] Implement -fzero-call-used-regs option

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added inline comments.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:6108
+  let Content = [{
+This attribute, when attached to a funcditon, causes the compiler to zero a
+subset of all call-used registers before the function returns. It's used to
----------------
s/funcditon/function/

`:set spell` in vim.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:6116
+
+- ``skip`` doesn't zero any call-used registers.
+- ``used`` only zeros call-used registers used in the function. By ``used``, we
----------------
Might be worth a note that `skip` is helpful for disabling previously occurring instances of the command line flag.  Sometimes toolchains have wrapper scripts that force on flags, or are built with the implicit default changed, so being able to explicitly disable a feature can be helpful.


================
Comment at: clang/lib/CodeGen/CGCall.cpp:2207
+          "zero-call-used-regs",
+          ZeroCallUsedRegsAttr::ConvertZeroCallUsedRegsKindToStr(Kind));
+    }
----------------
The method name has some duplicate information. Perhaps `KindToStr` would be more concise?


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7584
+
+  ZeroCallUsedRegsAttr::ZeroCallUsedRegsKind kind;
+  if (!ZeroCallUsedRegsAttr::ConvertStrToZeroCallUsedRegsKind(KindStr, kind)) {
----------------
capital K `Kind`?


================
Comment at: clang/test/CodeGen/zero-call-used-regs.c:219
+
+#define __zero_call_used_regs(...) __attribute__((zero_call_used_regs(__VA_ARGS__)))
+
----------------
I don't think any of the expansion sites use a variable number of parameters, or more than one?


================
Comment at: clang/test/Sema/zero_call_used_regs.c:6
+void failure() _zero_call_used_regs();                   // expected-error {{takes one argument}}
+void failure() _zero_call_used_regs("used", "used-gpr"); // expected-error {{takes one argument}}
+void failure() _zero_call_used_regs(0);                  // expected-error {{requires a string}}
----------------
How about a test for:

```
void failure __attribute__((zero_call_used_reg("used"),zero_call_used_reg("used-gpr")));
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110869