pcc marked 6 inline comments as done.
pcc added inline comments.
================
Comment at: clang/test/CodeGen/cfi-icall-canonical-jump-tables.c:15
+// CHECK: define void @g({{.*}} [[ATTR2:#[0-9]+]]
+__attribute__((cfi_jump_table_canonical)) void g() {}
+
----------------
eugenis wrote:
> would it be more natural to spell it "cfi_canonical_jump_table" ?
> No strong feelings one way or the other.
Me neither. In the end it's probably a little better to be consistent with the
flag name, so I've renamed it.
================
Comment at: llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp:328
+ if (lowertypetests::isJumpTableCanonical(&F))
Linkage = CFL_Definition;
+ else if (F.hasExternalWeakLinkage())
----------------
eugenis wrote:
> Is this correct? Looks like it would cause function definitions with
> non-canonical jump tables to be exported as declarations. This could be
> desirable for LowerTypeTests, but could it affect other users negatively?
http://llvm-cs.pcc.me.uk/?q=%22cfi.functions%22
This metadata is only used by LowerTypeTests to decide how to create the jump
table (and by CrossDSOCFI, but it doesn't look at this field). So I don't think
it should affect anything else.
================
Comment at:
llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-functions-canonical-jump-tables.ll:4
+
+; CHECK: !"f1", i8 1
+; CHECK: !"f2", i8 1
----------------
eugenis wrote:
> Does this test check exported functions metadata? Could you add a check for
> the metadata name, or at least a comment to make this easier to understand in
> the future?
Yes, it's for exported functions. I've added a comment here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65629/new/
https://reviews.llvm.org/D65629
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
