This is an archive of the discontinued LLVM Phabricator instance.

[Passes] Don't run tail-call-elim in -O1
AbandonedPublic

Authored by aeubanks on Aug 16 2022, 10:33 AM.

Details

Summary

Following up from D130374.

-O1 explicitly tries to not run passes that hurt debuggability too much.
tail-call-elim hurts debuggability, so don't run it under -O1.

Diff Detail

Event Timeline

aeubanks created this revision.Aug 16 2022, 10:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 16 2022, 10:33 AM
aeubanks requested review of this revision.Aug 16 2022, 10:33 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 16 2022, 10:33 AM
spatel accepted this revision.Aug 16 2022, 10:52 AM

LGTM
These clang tests are just awful, but I don't have the patience to fix them...

llvm/lib/Passes/PassBuilderPipelines.cpp
244–245

Update this comment? IIUC, we are decisively saying that tail call elim is not worth the impact to debuggability for -O1. That way, someone -- possibly me again :) -- won't try to unknowingly change this.

This revision is now accepted and ready to land.Aug 16 2022, 10:52 AM

Are you concerned about tail-call marking, or the recursive call->loop transform?

aeubanks updated this revision to Diff 453070.Aug 16 2022, 10:56 AM

delete comment

Are you concerned about tail-call marking, or the recursive call->loop transform?

specifically tail call marking

we have symbolizers that stopped displaying some frames with this change that we expect to work under -O1

If you're specifically concerned about sibcall (call->jmp) optimization in the backend, it might be better to adjust the backend to avoid sibcalls at -O1, as opposed to messing with optimization passes. (i.e. make -fno-optimize-sibling-calls the default at -O1.) "tail" markings are useful for other purposes, like alias analysis, and I don't really want every optimization pass/frontend that might add "tail" markings to worry about the optimization level.

aeubanks abandoned this revision.Aug 18 2022, 1:55 PM

will use Eli's suggestion instead