[PATCH] D127007: [llvm] Add 'G' to augmentation string for MTE instrumented functions.

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jun 5 14:38:18 PDT 2022


MaskRay added a comment.

Thanks for starting the GCC thread! I think the implementation is reasonable.

> [llvm] Add 'G' to augmentation string for MTE instrumented functions.

`[MC]` may be more specific. The convention is to avoid trailing dot in the subject line.

> This was agreed on in https://lists.llvm.org/pipermail/llvm-dev/2020-May/141345.html.

Avoid trailing `.` in an URI. Many tools consider `.` part of the URI.

Try summarizing the main point of the thread and place it in the commit message (summary).



================
Comment at: llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp:1294
+
+      if (getFunctionCFISectionType(*MF) == CFISection::None)
+        return;
----------------
There are too many blank lines.

Use
```
if (getFunctionCFISectionType(*MF) != CFISection::None)
  OutStreamer->emitCFIMTETaggedFrame();
```


================
Comment at: llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:6532
+bool AArch64AsmParser::parseDirectiveCFIMTETaggedFrame() {
+  if (parseToken(AsmToken::EndOfStatement,
+                 "unexpected token in '.cfi_mte_tagged_frame'"))
----------------
```
if (parseEOL())
  return true;
```

I've updated other `unexpected *** in ... directive` to use the prevailing style `expected newline`.


================
Comment at: llvm/test/CodeGen/AArch64/stack-tagging-cfi.ll:3
+
+declare void @use32(i32*)
+
----------------
`i32*` => `ptr`


================
Comment at: llvm/test/CodeGen/AArch64/stack-tagging-cfi.ll:7
+entry:
+; CHECK-LABEL: WithUnwind
+; CHECK: .cfi_mte_tagged_frame
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127007



More information about the llvm-commits mailing list