[PATCH] D108478: [llvm][IR] Add no_cfi constant

[Sami Tolvanen via Phabricator via llvm-commits]

samitolvanen added a comment.

In D108478#2967870 <https://reviews.llvm.org/D108478#2967870>, @nickdesaulniers wrote:

> I think you might want to update the syntax files for the various text editors. See:
>
> - llvm/utils/vim/syntax/llvm.vim
> - llvm/utils/emacs/llvm-mode.el
> - llvm/utils/kate/llvm.xml
> - llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml

I was looking at what was done with `dso_local_equivalent`, which is similar constant type, and it looks like that's only in `emacs/llvm-mode.el`. I'm not sure how kate or vscode would display this keyword, but I added it to vim and emacs for now.



================
Comment at: llvm/test/CodeGen/X86/nocfivalue.ll:1
+; RUN: opt -S -lowertypetests < %s | llc -asm-verbose=false | FileCheck %s
+
----------------
nickdesaulniers wrote:
> Do we need `<`? If not, let's remove.
> 
> How does `-asm-verbose=false` change this test?
> How does `-asm-verbose=false` change this test?

It removes some comments from the output, which doesn't affect this test, but I suppose might reduce the probability of future breakages. I followed the example of other similar tests here.


================
Comment at: llvm/test/CodeGen/X86/nocfivalue.ll:15-25
+define void @f1() !type !0 {
+  ret void
+}
+
+define internal void @f2() !type !0 {
+  ret void
+}
----------------
nickdesaulniers wrote:
> Can we change these to be simply declarations rather then empty function definitions?
> Can we change these to be simply declarations rather then empty function definitions?

Only `@f1` it seems. Using `declare` for an internal function results in an error, and the "cfi-canonical-jump-table" attribute is also ignored for a declaration.


================
Comment at: llvm/test/Transforms/LowerTypeTests/nocfivalue.ll:9-22
+; CHECK: define void @f1()
+define void @f1() !type !0 {
+  ret void
+}
+
+; CHECK: define internal void @f2()
+define internal void @f2() !type !0 {
----------------
nickdesaulniers wrote:
> declare?
> declare?

Only `@f1` again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108478