Skip to content

[EHStreamer] nounwind on call-sites and builtins is ignored #107195

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nikic opened this issue Sep 4, 2024 · 2 comments
Open

[EHStreamer] nounwind on call-sites and builtins is ignored #107195

nikic opened this issue Sep 4, 2024 · 2 comments

Comments

@nikic
Copy link
Contributor

nikic commented Sep 4, 2024

https://llvm.godbolt.org/z/4hhf36Wf8

declare void @personality()

declare void @nounwind_decl() nounwind
declare void @nounwind_call()

define void @test_nounwind_decl() personality ptr @personality {
  call void @nounwind_decl()
  ret void
}

define void @test_nounwind_call() personality ptr @personality {
  call void @nounwind_call() nounwind
  ret void
}

define void @test_memset(ptr %p) personality ptr @personality {
  call void @llvm.memset(ptr %p, i8 0, i64 1000, i1 false)
  ret void
}

In this example, the gcc_except_table section for test_nounwind_decl does not contain information about the call (as the declaration is nounwind), but the sections for test_nounwind_call and test_memset do contain it.

This is because

bool EHStreamer::callToNoUnwindFunction(const MachineInstr *MI) {
looks at the attributes of a Function operand of the call. This does not work if the call-site was marked nounwind, or if it's a libcall lowering, e.g. of an intrinsic.

@nikic
Copy link
Contributor Author

nikic commented Sep 4, 2024

@efriedma-quic Before I spend time working on this, do you think adding a NoUnwind flag to MachineInstr would make sense?

From poking around a bit, I think the pathway to preserving it would be via CallLoweringInfo -> NodeExtraInfo -> MachineInstr (the last step in ScheduleDAG).

@efriedma-quic
Copy link
Collaborator

The issue here optimizing the size of the unwind table for functions that contain both nounwind and non-nounwind calls? If we correctly detect the nounwind calls, we can skip adding entries to the table for the relevant basic blocks. (Dropping the nounwind is conservatively correct, but not optimal. And this is completely irrelevant for function definitions marked nounwind.)

I guess it makes sense to mark that on the MachineInstr for the call; EHStreamer::callToNoUnwindFunction is trying to rederive information we should already have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants