This is an archive of the discontinued LLVM Phabricator instance.

[cfi] Emit jump tables as a function-level inline asm
ClosedPublic

Authored by eugenis on Dec 20 2016, 6:10 PM.

Details

Summary

Use a dummy private function with inline asm calls instead of module level asm blocks for CFI jumptables.

The main advantage is that now jumptable codegen can be affected by the function attributes (like target_cpu on ARM). Module level asm gets the default subtarget based on the target triple, which is often not good enough.

This change also uses asm constraints/arguments to reference jumptable targets and aliases directly. We no longer do asm name mangling in an IR pass.

Diff Detail

Repository
rL LLVM

Event Timeline

eugenis updated this revision to Diff 82189.Dec 20 2016, 6:10 PM
eugenis retitled this revision from to [cfi] Emit jump tables as a function-level inline asm.
eugenis updated this object.
eugenis added a reviewer: pcc.
eugenis set the repository for this revision to rL LLVM.
eugenis added a subscriber: echristo.
eugenis updated this revision to Diff 82273.Dec 21 2016, 2:23 PM

Fix asm for non-ex86 targets.
${0:c} is x86-specific. Other targets emit a function label w/o "$" with simple $0 substitution (x86 emits $label in that case).

eugenis updated this revision to Diff 82299.Dec 21 2016, 6:06 PM

Replaced asm aliases with IR aliases based on the function that holds the jumptable inline asm.

pcc accepted this revision.Dec 21 2016, 7:54 PM
pcc edited edge metadata.

Thanks, this certainly seems nicer than the previous approach and should map quite cleanly into whichever IR-level representation we ultimately end up using for this. Just a few minor comments, then LGTM.

lib/Transforms/IPO/LowerTypeTests.cpp
263–264

I think you can remove this field now. Same for the include above.

694–695

Please update this comment.

815

Is this a string attribute? I thought it was an enum (llvm::Attribute::Naked). Perhaps this is just getting things right by chance (i.e. the backend figures out that it does not need a prologue).

This revision is now accepted and ready to land.Dec 21 2016, 7:54 PM
eugenis updated this revision to Diff 82371.Dec 22 2016, 2:20 PM
eugenis edited edge metadata.
eugenis marked 2 inline comments as done.
eugenis added inline comments.
lib/Transforms/IPO/LowerTypeTests.cpp
815

Yes, you are right. There is no prologue on any of the supported targets. Even on win32.
Strangely enough, win32 target is not very fond of inline assembly in a naked function (see https://llvm.org/bugs/show_bug.cgi?id=28641#c3).

Adding llvm::Attribute::Naked on all non-windows targets.

If a prologue is added at some point, our runtime tests will surely catch it, so I think we are good.

This revision was automatically updated to reflect the committed changes.