Allow users to preserve frame pointers in Halide-generated code

[abadams]

This can be done as an attribute on the llvm::Function: function->setFnAttr("frame-pointer", "all");

[mcourteaux]

Or with llvm::Module::setFramePointer(llvm::FramePointerKind::All) when we init the module.

[mcourteaux]

Example of a broken off backtrace:

[mcourteaux]

I tried this. Can confirm what @abadams suggested works, and what I suggested doesn't work with the following error when running the generator:

module flag identifiers must be unique (or of 'require' type)
!"frame-pointer"

The patch is simple:

diff --git a/src/CodeGen_Internal.cpp b/src/CodeGen_Internal.cpp
index 505e2b418..491712feb 100644
--- a/src/CodeGen_Internal.cpp
+++ b/src/CodeGen_Internal.cpp
@@ -700,6 +700,7 @@ void set_function_attributes_from_halide_target_options(llvm::Function &fn) {
     fn.addFnAttr("target-cpu", mcpu_target);
     fn.addFnAttr("tune-cpu", mcpu_tune);
     fn.addFnAttr("target-features", mattrs);
+    fn.addFnAttr("frame-pointer", "all");
 
     // Halide-generated IR is not exception-safe.
     // No exception should unwind out of Halide functions.

Finally a working backtrace 🥳 :

[slomp]

The big question is: should frame pointers by enabled by default, instead of opt-in?
As per @zvookin comment in the meeting, the callstacks around Halide code are likely not too deep, and probably lengthy in terms of execution time, so the overhead of emitting frame pointer should be negligible (for architectures with plenty of registers, at least).

I don't think we need to make this decision now, so merging #8703 is good to go regardless of how the discussion unrolls here.

I was re-educating myself on the topic, so here are some references in case people are interested:

• General overview (and support for having frame pointers): https://www.brendangregg.com/blog/2024-03-17/the-return-of-the-frame-pointers.html
• Benchmarks (circa 2022) in support for omitting frame pointers (though they should be taken with a large Himalayan salt rock, in my opinion) : https://www.phoronix.com/review/fedora-frame-pointer
• More detailed overview and where the industry stands on the matter: https://community.ibm.com/community/user/blogs/kevin-grigorenko1/2023/02/21/lessons-from-the-field-26-frame-pointers-and-why-t
• The Fedora study case: https://fedoraproject.org/wiki/Changes/fno-omit-frame-pointer#Benchmarking_of_the_performance_impact
• More overview, with details about the .eh_frames alternative (piggy back on C++ exception handlers): https://israelo.io/blog/ehframes/
• A more neutral take, and some alternatives (but maybe impractical): https://developers.redhat.com/articles/2024/10/30/limitations-frame-pointer-unwinding#alternatives_to_frame_pointer_unwinding
• An interesting paper from Microsoft circa 2009 that briefly touches upon the impact of frame-pointer omission on Windows: https://www.microsoft.com/en-us/research/wp-content/uploads/2016/02/sosp153-glerum-web.pdf

[mcourteaux]

the overhead of emitting frame pointer should be negligible

I haven't gone through your resources yet, but my thoughts are that keeping the frame pointer sacrifices a general purpose register. I'm open to invert the feature flag logic: ElimFramePointer.

[slomp]

keeping the frame pointer sacrifices a general purpose register.

Yes, but on 32bit Intel/x86: tthere are only 7 registers, so dedicating one to the frame pointer is a big deal.